| Message ID | 20190617125724.1616165-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: hide do_bpf_send_signal when unused | expand |
On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <arnd@arndb.de> wrote: > > When CONFIG_MODULES is disabled, this function is never called: > > kernel/trace/bpf_trace.c:581:13: error: 'do_bpf_send_signal' defined but not used [-Werror=unused-function] hmm. it should work just fine without modules. the bug is somewhere else.
On Mon, 17 Jun 2019 08:26:29 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > When CONFIG_MODULES is disabled, this function is never called: > > > > kernel/trace/bpf_trace.c:581:13: error: 'do_bpf_send_signal' defined but not used [-Werror=unused-function] > > hmm. it should work just fine without modules. > the bug is somewhere else. From what I see, the only use of do_bpf_send_signal is within a #ifdef CONFIG_MODULES, which means that you will get a warning about a static unused when CONFIG_MODULES is not defined. In kernel/trace/bpf_trace.c we have: static void do_bpf_send_signal(struct irq_work *entry) [..] #ifdef CONFIG_MODULES [..] for_each_possible_cpu(cpu) { work = per_cpu_ptr(&send_signal_work, cpu); init_irq_work(&work->irq_work, do_bpf_send_signal); <-- on use of do_bpf_send_signal } [..] #endif /* CONFIG_MODULES */ The bug (really just a warning) reported is exactly here. -- Steve
On Mon, 2019-06-17 at 19:09 -0400, Steven Rostedt wrote: > On Mon, 17 Jun 2019 08:26:29 -0700 > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > > On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > When CONFIG_MODULES is disabled, this function is never called: > > > > > > kernel/trace/bpf_trace.c:581:13: error: 'do_bpf_send_signal' defined but not used [-Werror=unused-function] > > > > hmm. it should work just fine without modules. > > the bug is somewhere else. > > From what I see, the only use of do_bpf_send_signal is within a > #ifdef CONFIG_MODULES, which means that you will get a warning about a > static unused when CONFIG_MODULES is not defined. > > In kernel/trace/bpf_trace.c we have: > > static void do_bpf_send_signal(struct irq_work *entry) > > [..] > > #ifdef CONFIG_MODULES > > [..] > > for_each_possible_cpu(cpu) { > work = per_cpu_ptr(&send_signal_work, cpu); > init_irq_work(&work->irq_work, do_bpf_send_signal); <-- on use of do_bpf_send_signal > } > [..] > #endif /* CONFIG_MODULES */ > > The bug (really just a warning) reported is exactly here. I don't think bpf_send_signal is tied to modules at all; send_signal_irq_work_init and the corresponding initcall should be moved outside that #ifdef. > > -- Steve
On Mon, Jun 17, 2019 at 4:13 PM Matt Mullins <mmullins@fb.com> wrote: > > > > The bug (really just a warning) reported is exactly here. > > I don't think bpf_send_signal is tied to modules at all; > send_signal_irq_work_init and the corresponding initcall should be > moved outside that #ifdef. right. I guess send_signal_irq_work_init was accidentally placed after bpf_event_init and happened to be within that ifdef. Should definitely be outside.
On 6/17/19 5:18 PM, Steven Rostedt wrote: > On Mon, 17 Jun 2019 16:27:33 -0700 > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > >> On Mon, Jun 17, 2019 at 4:13 PM Matt Mullins <mmullins@fb.com> wrote: >>>> >>>> The bug (really just a warning) reported is exactly here. >>> >>> I don't think bpf_send_signal is tied to modules at all; >>> send_signal_irq_work_init and the corresponding initcall should be >>> moved outside that #ifdef. >> >> right. I guess send_signal_irq_work_init was accidentally placed >> after bpf_event_init and happened to be within that ifdef. >> Should definitely be outside. > > So Arnd did find a bug. Just the wrong solution ;-) > > -- Steve Hi, Arnd, The following change can fix the issue. diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index c102c240bb0b..ca1255d14576 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1431,6 +1431,20 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id, return err; } +static int __init send_signal_irq_work_init(void) +{ + int cpu; + struct send_signal_irq_work *work; + + for_each_possible_cpu(cpu) { + work = per_cpu_ptr(&send_signal_work, cpu); + init_irq_work(&work->irq_work, do_bpf_send_signal); + } + return 0; +} + +subsys_initcall(send_signal_irq_work_init); + #ifdef CONFIG_MODULES static int bpf_event_notify(struct notifier_block *nb, unsigned long op, void *module) @@ -1478,18 +1492,5 @@ static int __init bpf_event_init(void) return 0; } -static int __init send_signal_irq_work_init(void) -{ - int cpu; - struct send_signal_irq_work *work; - - for_each_possible_cpu(cpu) { - work = per_cpu_ptr(&send_signal_work, cpu); - init_irq_work(&work->irq_work, do_bpf_send_signal); - } - return 0; -} - fs_initcall(bpf_event_init); -subsys_initcall(send_signal_irq_work_init); #endif /* CONFIG_MODULES */ Could you submit a new revision? Thanks! Yonghong
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index c102c240bb0b..b1a814e2d451 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -602,6 +602,7 @@ struct send_signal_irq_work { static DEFINE_PER_CPU(struct send_signal_irq_work, send_signal_work); +#ifdef CONFIG_MODULES static void do_bpf_send_signal(struct irq_work *entry) { struct send_signal_irq_work *work; @@ -609,6 +610,7 @@ static void do_bpf_send_signal(struct irq_work *entry) work = container_of(entry, struct send_signal_irq_work, irq_work); group_send_sig_info(work->sig, SEND_SIG_PRIV, work->task, PIDTYPE_TGID); } +#endif BPF_CALL_1(bpf_send_signal, u32, sig) {
When CONFIG_MODULES is disabled, this function is never called: kernel/trace/bpf_trace.c:581:13: error: 'do_bpf_send_signal' defined but not used [-Werror=unused-function] Add another #ifdef around it. Fixes: 8b401f9ed244 ("bpf: implement bpf_send_signal() helper") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- kernel/trace/bpf_trace.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.20.0