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 <[email protected]>
---
kernel/trace/bpf_trace.c | 2 ++
1 file changed, 2 insertions(+)
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)
{
--
2.20.0
On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <[email protected]> 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 <[email protected]> wrote:
> On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <[email protected]> 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 <[email protected]> wrote:
>
> > On Mon, Jun 17, 2019 at 5:59 AM Arnd Bergmann <[email protected]> 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 <[email protected]> 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 Mon, 17 Jun 2019 16:27:33 -0700
Alexei Starovoitov <[email protected]> wrote:
> On Mon, Jun 17, 2019 at 4:13 PM Matt Mullins <[email protected]> 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
On 6/17/19 5:18 PM, Steven Rostedt wrote:
> On Mon, 17 Jun 2019 16:27:33 -0700
> Alexei Starovoitov <[email protected]> wrote:
>
>> On Mon, Jun 17, 2019 at 4:13 PM Matt Mullins <[email protected]> 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