Message ID | 20180817065500.16899-1-mpe@ellerman.id.au (mailing list archive) |
---|---|
State | Accepted |
Commit | 997dd26cb3c8b7c9b8765751cc1491ad33b2024f |
Headers | show |
Series | powerpc/traps: Avoid rate limit messages from show unhandled signals | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
Hi, Michael. On Fri, Aug 17, 2018 at 04:55:00PM +1000, Michael Ellerman wrote: > In the recent commit to add an explicit ratelimit state when showing > unhandled signals, commit 35a52a10c3ac ("powerpc/traps: Use an > explicit ratelimit state for show_signal_msg()"), I put the check of > show_unhandled_signals and the ratelimit state before the call to > unhandled_signal() so as to avoid unnecessarily calling the latter > when show_unhandled_signals is false. > > However that causes us to check the ratelimit state on every call, so > if we take a lot of *handled* signals that has the effect of making > the ratelimit code print warnings that callbacks have been suppressed > when they haven't. > > So rearrange the code so that we check show_unhandled_signals first, > then call unhandled_signal() and finally check the ratelimit state. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Nice catch. Thanks for patching it. Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > --- > arch/powerpc/kernel/traps.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c > index 070e96f1773a..c85adb858271 100644 > --- a/arch/powerpc/kernel/traps.c > +++ b/arch/powerpc/kernel/traps.c > @@ -315,22 +315,21 @@ void user_single_step_siginfo(struct task_struct *tsk, > info->si_addr = (void __user *)regs->nip; > } > > -static bool show_unhandled_signals_ratelimited(void) > +static void show_signal_msg(int signr, struct pt_regs *regs, int code, > + unsigned long addr) > { > static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, > DEFAULT_RATELIMIT_BURST); > - return show_unhandled_signals && __ratelimit(&rs); > -} > > -static void show_signal_msg(int signr, struct pt_regs *regs, int code, > - unsigned long addr) > -{ > - if (!show_unhandled_signals_ratelimited()) > + if (!show_unhandled_signals) > return; > > if (!unhandled_signal(current, signr)) > return; > > + if (!__ratelimit(&rs)) > + return; > + > pr_info("%s[%d]: %s (%d) at %lx nip %lx lr %lx code %x", > current->comm, current->pid, signame(signr), signr, > addr, regs->nip, regs->link, code); > -- > 2.17.1 >
On Fri, 2018-08-17 at 06:55:00 UTC, Michael Ellerman wrote: > In the recent commit to add an explicit ratelimit state when showing > unhandled signals, commit 35a52a10c3ac ("powerpc/traps: Use an > explicit ratelimit state for show_signal_msg()"), I put the check of > show_unhandled_signals and the ratelimit state before the call to > unhandled_signal() so as to avoid unnecessarily calling the latter > when show_unhandled_signals is false. > > However that causes us to check the ratelimit state on every call, so > if we take a lot of *handled* signals that has the effect of making > the ratelimit code print warnings that callbacks have been suppressed > when they haven't. > > So rearrange the code so that we check show_unhandled_signals first, > then call unhandled_signal() and finally check the ratelimit state. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Reviewed-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> Applied to powerpc next. https://git.kernel.org/powerpc/c/997dd26cb3c8b7c9b8765751cc1491 cheers
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index 070e96f1773a..c85adb858271 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -315,22 +315,21 @@ void user_single_step_siginfo(struct task_struct *tsk, info->si_addr = (void __user *)regs->nip; } -static bool show_unhandled_signals_ratelimited(void) +static void show_signal_msg(int signr, struct pt_regs *regs, int code, + unsigned long addr) { static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST); - return show_unhandled_signals && __ratelimit(&rs); -} -static void show_signal_msg(int signr, struct pt_regs *regs, int code, - unsigned long addr) -{ - if (!show_unhandled_signals_ratelimited()) + if (!show_unhandled_signals) return; if (!unhandled_signal(current, signr)) return; + if (!__ratelimit(&rs)) + return; + pr_info("%s[%d]: %s (%d) at %lx nip %lx lr %lx code %x", current->comm, current->pid, signame(signr), signr, addr, regs->nip, regs->link, code);
In the recent commit to add an explicit ratelimit state when showing unhandled signals, commit 35a52a10c3ac ("powerpc/traps: Use an explicit ratelimit state for show_signal_msg()"), I put the check of show_unhandled_signals and the ratelimit state before the call to unhandled_signal() so as to avoid unnecessarily calling the latter when show_unhandled_signals is false. However that causes us to check the ratelimit state on every call, so if we take a lot of *handled* signals that has the effect of making the ratelimit code print warnings that callbacks have been suppressed when they haven't. So rearrange the code so that we check show_unhandled_signals first, then call unhandled_signal() and finally check the ratelimit state. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/traps.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)