* [PATCH 2/3] module: Fix up module_notifier return values. [not found] <20190624091843.859714294@infradead.org> @ 2019-06-24 9:18 ` Peter Zijlstra 2019-06-24 14:01 ` Mathieu Desnoyers 2019-07-04 12:34 ` Robert Richter 2019-06-24 9:18 ` [PATCH 3/3] module: Properly propagate MODULE_STATE_COMING failure Peter Zijlstra 1 sibling, 2 replies; 10+ messages in thread From: Peter Zijlstra @ 2019-06-24 9:18 UTC (permalink / raw) To: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast, daniel, akpm, peterz Cc: Robert Richter, Steven Rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, Mathieu Desnoyers, Paul E. McKenney, Joel Fernandes (Google), Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf While auditing all module notifiers I noticed a whole bunch of fail wrt the return value. Notifiers have a 'special' return semantics. Cc: Robert Richter <rric@kernel.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Martin KaFai Lau <kafai@fb.com> Cc: Song Liu <songliubraving@fb.com> Cc: Yonghong Song <yhs@fb.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: oprofile-list@lists.sf.net Cc: linux-kernel@vger.kernel.org Cc: netdev@vger.kernel.org Cc: bpf@vger.kernel.org Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- drivers/oprofile/buffer_sync.c | 4 ++-- kernel/module.c | 9 +++++---- kernel/trace/bpf_trace.c | 8 ++++++-- kernel/trace/trace.c | 2 +- kernel/trace/trace_events.c | 2 +- kernel/trace/trace_printk.c | 4 ++-- kernel/tracepoint.c | 2 +- 7 files changed, 18 insertions(+), 13 deletions(-) --- a/drivers/oprofile/buffer_sync.c +++ b/drivers/oprofile/buffer_sync.c @@ -116,7 +116,7 @@ module_load_notify(struct notifier_block { #ifdef CONFIG_MODULES if (val != MODULE_STATE_COMING) - return 0; + return NOTIFY_DONE; /* FIXME: should we process all CPU buffers ? */ mutex_lock(&buffer_mutex); @@ -124,7 +124,7 @@ module_load_notify(struct notifier_block add_event_entry(MODULE_LOADED_CODE); mutex_unlock(&buffer_mutex); #endif - return 0; + return NOTIFY_OK; } --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1302,10 +1302,11 @@ static int bpf_event_notify(struct notif { struct bpf_trace_module *btm, *tmp; struct module *mod = module; + int ret = 0; if (mod->num_bpf_raw_events == 0 || (op != MODULE_STATE_COMING && op != MODULE_STATE_GOING)) - return 0; + goto out; mutex_lock(&bpf_module_mutex); @@ -1315,6 +1316,8 @@ static int bpf_event_notify(struct notif if (btm) { btm->module = module; list_add(&btm->list, &bpf_trace_modules); + } else { + ret = -ENOMEM; } break; case MODULE_STATE_GOING: @@ -1330,7 +1333,8 @@ static int bpf_event_notify(struct notif mutex_unlock(&bpf_module_mutex); - return 0; +out: + return notifier_from_errno(ret); } static struct notifier_block bpf_module_nb = { --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -8685,7 +8685,7 @@ static int trace_module_notify(struct no break; } - return 0; + return NOTIFY_OK; } static struct notifier_block trace_module_nb = { --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c @@ -2450,7 +2450,7 @@ static int trace_module_notify(struct no mutex_unlock(&trace_types_lock); mutex_unlock(&event_mutex); - return 0; + return NOTIFY_OK; } static struct notifier_block trace_module_nb = { --- a/kernel/trace/trace_printk.c +++ b/kernel/trace/trace_printk.c @@ -95,7 +95,7 @@ static int module_trace_bprintk_format_n if (val == MODULE_STATE_COMING) hold_module_trace_bprintk_format(start, end); } - return 0; + return NOTIFY_OK; } /* @@ -173,7 +173,7 @@ __init static int module_trace_bprintk_format_notify(struct notifier_block *self, unsigned long val, void *data) { - return 0; + return NOTIFY_OK; } static inline const char ** find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos) --- a/kernel/tracepoint.c +++ b/kernel/tracepoint.c @@ -538,7 +538,7 @@ static int tracepoint_module_notify(stru case MODULE_STATE_UNFORMED: break; } - return ret; + return notifier_from_errno(ret); } static struct notifier_block tracepoint_module_nb = { ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 9:18 ` [PATCH 2/3] module: Fix up module_notifier return values Peter Zijlstra @ 2019-06-24 14:01 ` Mathieu Desnoyers 2019-06-24 15:52 ` Joel Fernandes 2019-06-24 20:58 ` Frank Ch. Eigler 2019-07-04 12:34 ` Robert Richter 1 sibling, 2 replies; 10+ messages in thread From: Mathieu Desnoyers @ 2019-06-24 14:01 UTC (permalink / raw) To: Peter Zijlstra, Frank Ch. Eigler Cc: Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, Robert Richter, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@infradead.org wrote: > While auditing all module notifiers I noticed a whole bunch of fail > wrt the return value. Notifiers have a 'special' return semantics. > > Cc: Robert Richter <rric@kernel.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Martin KaFai Lau <kafai@fb.com> > Cc: Song Liu <songliubraving@fb.com> > Cc: Yonghong Song <yhs@fb.com> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: oprofile-list@lists.sf.net > Cc: linux-kernel@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Thanks Peter for looking into this, especially considering your endless love for kernel modules! ;) It's not directly related to your changes, but I notice that kernel/trace/trace_printk.c:hold_module_trace_bprintk_format() appears to leak memory. Am I missing something ? With respect to your changes: Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> I have a similar erroneous module notifier return value pattern in lttng-modules as well. I'll go fix it right away. CCing Frank Eigler from SystemTAP which AFAIK use a copy of lttng-tracepoint.c in their project, which should be fixed as well. I'm pasting the lttng-modules fix below. Thanks! Mathieu -- commit 5eac9d146a7d947f0f314c4f7103c92cbccaeaf3 Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Date: Mon Jun 24 09:43:45 2019 -0400 Fix: lttng-tracepoint module notifier should return NOTIFY_OK Module notifiers should return NOTIFY_OK on success rather than the value 0. The return value 0 does not seem to have any ill side-effects in the notifier chain caller, but it is preferable to respect the API requirements in case this changes in the future. Notifiers can encapsulate a negative errno value with notifier_from_errno(), but this is not needed by the LTTng tracepoint notifier. The approach taken in this notifier is to just print a console warning on error, because tracing failure should not prevent loading a module. So we definitely do not want to stop notifier iteration. Returning an error without stopping iteration is not really that useful, because only the return value of the last callback is returned to notifier chain caller. Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> diff --git a/lttng-tracepoint.c b/lttng-tracepoint.c index bbb2c7a4..8298b397 100644 --- a/lttng-tracepoint.c +++ b/lttng-tracepoint.c @@ -256,7 +256,7 @@ int lttng_tracepoint_coming(struct tp_module *tp_mod) } } mutex_unlock(<tng_tracepoint_mutex); - return 0; + return NOTIFY_OK; } static -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 14:01 ` Mathieu Desnoyers @ 2019-06-24 15:52 ` Joel Fernandes 2019-06-24 17:50 ` Mathieu Desnoyers 2019-06-24 20:58 ` Frank Ch. Eigler 1 sibling, 1 reply; 10+ messages in thread From: Joel Fernandes @ 2019-06-24 15:52 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Peter Zijlstra, Frank Ch. Eigler, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, Robert Richter, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf On Mon, Jun 24, 2019 at 10:01:04AM -0400, Mathieu Desnoyers wrote: > ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@infradead.org wrote: > > > While auditing all module notifiers I noticed a whole bunch of fail > > wrt the return value. Notifiers have a 'special' return semantics. > > > > Cc: Robert Richter <rric@kernel.org> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > Cc: Martin KaFai Lau <kafai@fb.com> > > Cc: Song Liu <songliubraving@fb.com> > > Cc: Yonghong Song <yhs@fb.com> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: oprofile-list@lists.sf.net > > Cc: linux-kernel@vger.kernel.org > > Cc: netdev@vger.kernel.org > > Cc: bpf@vger.kernel.org > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Thanks Peter for looking into this, especially considering your > endless love for kernel modules! ;) > > It's not directly related to your changes, but I notice that > kernel/trace/trace_printk.c:hold_module_trace_bprintk_format() > appears to leak memory. Am I missing something ? Could you elaborate? Do you mean there is no MODULE_STATE_GOING notifier check? If that's what you mean then I agree, there should be some place where the format structures are freed when the module is unloaded no? > > With respect to your changes: > Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Looks good to me too. Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> Could we CC stable so that the fix is propagated to older kernels? thanks, - Joel > I have a similar erroneous module notifier return value pattern > in lttng-modules as well. I'll go fix it right away. CCing > Frank Eigler from SystemTAP which AFAIK use a copy of > lttng-tracepoint.c in their project, which should be fixed > as well. I'm pasting the lttng-modules fix below. > > Thanks! > > Mathieu > > -- > > commit 5eac9d146a7d947f0f314c4f7103c92cbccaeaf3 > Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Date: Mon Jun 24 09:43:45 2019 -0400 > > Fix: lttng-tracepoint module notifier should return NOTIFY_OK > > Module notifiers should return NOTIFY_OK on success rather than the > value 0. The return value 0 does not seem to have any ill side-effects > in the notifier chain caller, but it is preferable to respect the API > requirements in case this changes in the future. > > Notifiers can encapsulate a negative errno value with > notifier_from_errno(), but this is not needed by the LTTng tracepoint > notifier. > > The approach taken in this notifier is to just print a console warning > on error, because tracing failure should not prevent loading a module. > So we definitely do not want to stop notifier iteration. Returning > an error without stopping iteration is not really that useful, because > only the return value of the last callback is returned to notifier chain > caller. > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > diff --git a/lttng-tracepoint.c b/lttng-tracepoint.c > index bbb2c7a4..8298b397 100644 > --- a/lttng-tracepoint.c > +++ b/lttng-tracepoint.c > @@ -256,7 +256,7 @@ int lttng_tracepoint_coming(struct tp_module *tp_mod) > } > } > mutex_unlock(<tng_tracepoint_mutex); > - return 0; > + return NOTIFY_OK; > } > > static > > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 15:52 ` Joel Fernandes @ 2019-06-24 17:50 ` Mathieu Desnoyers 0 siblings, 0 replies; 10+ messages in thread From: Mathieu Desnoyers @ 2019-06-24 17:50 UTC (permalink / raw) To: Joel Fernandes, Google Cc: Peter Zijlstra, Frank Ch. Eigler, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, Robert Richter, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf ----- On Jun 24, 2019, at 11:52 AM, Joel Fernandes, Google joel@joelfernandes.org wrote: > On Mon, Jun 24, 2019 at 10:01:04AM -0400, Mathieu Desnoyers wrote: >> ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@infradead.org wrote: >> >> > While auditing all module notifiers I noticed a whole bunch of fail >> > wrt the return value. Notifiers have a 'special' return semantics. >> > >> > Cc: Robert Richter <rric@kernel.org> >> > Cc: Steven Rostedt <rostedt@goodmis.org> >> > Cc: Ingo Molnar <mingo@redhat.com> >> > Cc: Alexei Starovoitov <ast@kernel.org> >> > Cc: Daniel Borkmann <daniel@iogearbox.net> >> > Cc: Martin KaFai Lau <kafai@fb.com> >> > Cc: Song Liu <songliubraving@fb.com> >> > Cc: Yonghong Song <yhs@fb.com> >> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> >> > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org> >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > Cc: Thomas Gleixner <tglx@linutronix.de> >> > Cc: oprofile-list@lists.sf.net >> > Cc: linux-kernel@vger.kernel.org >> > Cc: netdev@vger.kernel.org >> > Cc: bpf@vger.kernel.org >> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> >> >> Thanks Peter for looking into this, especially considering your >> endless love for kernel modules! ;) >> >> It's not directly related to your changes, but I notice that >> kernel/trace/trace_printk.c:hold_module_trace_bprintk_format() >> appears to leak memory. Am I missing something ? > > Could you elaborate? Do you mean there is no MODULE_STATE_GOING notifier > check? If that's what you mean then I agree, there should be some place > where the format structures are freed when the module is unloaded no? Yes, the lack of GOING notifier is worrying considering that GOING performs memory allocation. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 14:01 ` Mathieu Desnoyers 2019-06-24 15:52 ` Joel Fernandes @ 2019-06-24 20:58 ` Frank Ch. Eigler 2019-06-25 7:42 ` Peter Zijlstra 1 sibling, 1 reply; 10+ messages in thread From: Frank Ch. Eigler @ 2019-06-24 20:58 UTC (permalink / raw) To: Mathieu Desnoyers Cc: Peter Zijlstra, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, Robert Richter, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf Hi - > > While auditing all module notifiers I noticed a whole bunch of fail > > wrt the return value. Notifiers have a 'special' return semantics. From peterz's comments, the patches, it's not obvious to me how one is to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a routine success. > [...] > I have a similar erroneous module notifier return value pattern > in lttng-modules as well. I'll go fix it right away. CCing > Frank Eigler from SystemTAP which AFAIK use a copy of > lttng-tracepoint.c in their project, which should be fixed > as well. I'm pasting the lttng-modules fix below. Sure, following suit. Thanks. - FChE ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 20:58 ` Frank Ch. Eigler @ 2019-06-25 7:42 ` Peter Zijlstra 2019-07-04 12:48 ` Robert Richter 0 siblings, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2019-06-25 7:42 UTC (permalink / raw) To: Frank Ch. Eigler Cc: Mathieu Desnoyers, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, Robert Richter, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf On Mon, Jun 24, 2019 at 04:58:10PM -0400, Frank Ch. Eigler wrote: > Hi - > > > > While auditing all module notifiers I noticed a whole bunch of fail > > > wrt the return value. Notifiers have a 'special' return semantics. > > From peterz's comments, the patches, it's not obvious to me how one is > to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a > routine success. I'm not sure either; what I think I choice was: - if I want to completely ignore the callback, use DONE (per the "Don't care" comment). - if we finished the notifier without error, use OK or notifier_from_errno(0). But yes, its a bit of a shit interface. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-25 7:42 ` Peter Zijlstra @ 2019-07-04 12:48 ` Robert Richter 0 siblings, 0 replies; 10+ messages in thread From: Robert Richter @ 2019-07-04 12:48 UTC (permalink / raw) To: Peter Zijlstra Cc: Frank Ch. Eigler, Mathieu Desnoyers, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann, Andrew Morton, rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, paulmck, Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf On 25.06.19 09:42:14, Peter Zijlstra wrote: > On Mon, Jun 24, 2019 at 04:58:10PM -0400, Frank Ch. Eigler wrote: > > From peterz's comments, the patches, it's not obvious to me how one is > > to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a > > routine success. > > I'm not sure either; what I think I choice was: > > - if I want to completely ignore the callback, use DONE (per the > "Don't care" comment). > > - if we finished the notifier without error, use OK or > notifier_from_errno(0). > > But yes, its a bit of a shit interface. It looks like it was rarely used in earlier kernels as some sort of error detection for the notifier calls, e.g.: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-int profile_handoff_task(struct task_struct * task) 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-{ 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- int ret; 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- read_lock(&handoff_lock); 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- ret = notifier_call_chain(&task_free_notifier, 0, task); 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c- read_unlock(&handoff_lock); 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c: return (ret == NOTIFY_OK) ? 1 : 0; 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-} So NOTIFY_OK was used to state there is no error, while NOTIFY_DONE says the notifier was executed and there might have been errors. The caller may distinguish the results then. -Robert ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] module: Fix up module_notifier return values. 2019-06-24 9:18 ` [PATCH 2/3] module: Fix up module_notifier return values Peter Zijlstra 2019-06-24 14:01 ` Mathieu Desnoyers @ 2019-07-04 12:34 ` Robert Richter 1 sibling, 0 replies; 10+ messages in thread From: Robert Richter @ 2019-07-04 12:34 UTC (permalink / raw) To: Peter Zijlstra Cc: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast, daniel, akpm, Steven Rostedt, Ingo Molnar, Martin KaFai Lau, Song Liu, Yonghong Song, Mathieu Desnoyers, Paul E. McKenney, Joel Fernandes (Google), Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf On 24.06.19 11:18:45, Peter Zijlstra wrote: > While auditing all module notifiers I noticed a whole bunch of fail > wrt the return value. Notifiers have a 'special' return semantics. > > Cc: Robert Richter <rric@kernel.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Martin KaFai Lau <kafai@fb.com> > Cc: Song Liu <songliubraving@fb.com> > Cc: Yonghong Song <yhs@fb.com> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: oprofile-list@lists.sf.net > Cc: linux-kernel@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > drivers/oprofile/buffer_sync.c | 4 ++-- > kernel/module.c | 9 +++++---- > kernel/trace/bpf_trace.c | 8 ++++++-- > kernel/trace/trace.c | 2 +- > kernel/trace/trace_events.c | 2 +- > kernel/trace/trace_printk.c | 4 ++-- > kernel/tracepoint.c | 2 +- > 7 files changed, 18 insertions(+), 13 deletions(-) Reviewed-by: Robert Richter <rric@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] module: Properly propagate MODULE_STATE_COMING failure [not found] <20190624091843.859714294@infradead.org> 2019-06-24 9:18 ` [PATCH 2/3] module: Fix up module_notifier return values Peter Zijlstra @ 2019-06-24 9:18 ` Peter Zijlstra 2019-06-24 11:07 ` Peter Zijlstra 1 sibling, 1 reply; 10+ messages in thread From: Peter Zijlstra @ 2019-06-24 9:18 UTC (permalink / raw) To: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast, daniel, akpm, peterz Cc: Martin KaFai Lau, Song Liu, Yonghong Song, netdev, bpf Now that notifiers got unbroken; use the proper interface to handle notifier errors and propagate them. There were already MODULE_STATE_COMING notifiers that failed; notably: - jump_label_module_notifier() - tracepoint_module_notify() - bpf_event_notify() By propagating this error, we fix those users. Cc: Jessica Yu <jeyu@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Martin KaFai Lau <kafai@fb.com> Cc: Song Liu <songliubraving@fb.com> Cc: Yonghong Song <yhs@fb.com> Cc: linux-kernel@vger.kernel.org Cc: netdev@vger.kernel.org Cc: bpf@vger.kernel.org Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- kernel/module.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) --- a/kernel/module.c +++ b/kernel/module.c @@ -3643,9 +3643,10 @@ static int prepare_coming_module(struct if (err) return err; - blocking_notifier_call_chain(&module_notify_list, - MODULE_STATE_COMING, mod); - return 0; + err = blocking_notifier_call_chain_error(&module_notify_list, + MODULE_STATE_COMING, MODULE_STATE_GOING, mod); + + return notifier_to_errno(err); } static int unknown_module_param_cb(char *param, char *val, const char *modname, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] module: Properly propagate MODULE_STATE_COMING failure 2019-06-24 9:18 ` [PATCH 3/3] module: Properly propagate MODULE_STATE_COMING failure Peter Zijlstra @ 2019-06-24 11:07 ` Peter Zijlstra 0 siblings, 0 replies; 10+ messages in thread From: Peter Zijlstra @ 2019-06-24 11:07 UTC (permalink / raw) To: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast, daniel, akpm Cc: Martin KaFai Lau, Song Liu, Yonghong Song, netdev, bpf On Mon, Jun 24, 2019 at 11:18:46AM +0200, Peter Zijlstra wrote: > Now that notifiers got unbroken; use the proper interface to handle > notifier errors and propagate them. > > There were already MODULE_STATE_COMING notifiers that failed; notably: > > - jump_label_module_notifier() > - tracepoint_module_notify() > - bpf_event_notify() > > By propagating this error, we fix those users. > > Cc: Jessica Yu <jeyu@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Martin KaFai Lau <kafai@fb.com> > Cc: Song Liu <songliubraving@fb.com> > Cc: Yonghong Song <yhs@fb.com> > Cc: linux-kernel@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/module.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3643,9 +3643,10 @@ static int prepare_coming_module(struct > if (err) > return err; > > - blocking_notifier_call_chain(&module_notify_list, > - MODULE_STATE_COMING, mod); > - return 0; > + err = blocking_notifier_call_chain_error(&module_notify_list, > + MODULE_STATE_COMING, MODULE_STATE_GOING, mod); > + > + return notifier_to_errno(err); > } Argh, I messed up that klp thing again :/ ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-04 12:48 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20190624091843.859714294@infradead.org> 2019-06-24 9:18 ` [PATCH 2/3] module: Fix up module_notifier return values Peter Zijlstra 2019-06-24 14:01 ` Mathieu Desnoyers 2019-06-24 15:52 ` Joel Fernandes 2019-06-24 17:50 ` Mathieu Desnoyers 2019-06-24 20:58 ` Frank Ch. Eigler 2019-06-25 7:42 ` Peter Zijlstra 2019-07-04 12:48 ` Robert Richter 2019-07-04 12:34 ` Robert Richter 2019-06-24 9:18 ` [PATCH 3/3] module: Properly propagate MODULE_STATE_COMING failure Peter Zijlstra 2019-06-24 11:07 ` Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).