* [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() @ 2025-05-17 9:30 Mike Rapoport 2025-05-19 8:48 ` Geert Uytterhoeven 2025-06-07 13:10 ` John Paul Adrian Glaubitz 0 siblings, 2 replies; 5+ messages in thread From: Mike Rapoport @ 2025-05-17 9:30 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Mike Rapoport, Rich Felker, Yoshinori Sato, linux-kernel, linux-sh, Mike Rapoport (Microsoft), kernel test robot From: "Mike Rapoport (Microsoft)" <rppt@kernel.org> kbuild reports the following warning: arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify': >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable] 412 | struct kprobe *p = NULL; | ^ The variable 'p' is indeed unused since the commit fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") Remove that variable along with 'kprobe_opcode_t *addr' which also becomes unused after 'p' is removed. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/ Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org> --- I don't know why the warning poped up only now, the code there didn't change for some time :/ arch/sh/kernel/kprobes.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/arch/sh/kernel/kprobes.c b/arch/sh/kernel/kprobes.c index 49c4ffd782d6..a250fb1b9420 100644 --- a/arch/sh/kernel/kprobes.c +++ b/arch/sh/kernel/kprobes.c @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) int __kprobes kprobe_exceptions_notify(struct notifier_block *self, unsigned long val, void *data) { - struct kprobe *p = NULL; struct die_args *args = (struct die_args *)data; int ret = NOTIFY_DONE; - kprobe_opcode_t *addr = NULL; struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); - addr = (kprobe_opcode_t *) (args->regs->pc); if (val == DIE_TRAP && args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) { if (!kprobe_running()) { @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, ret = NOTIFY_DONE; } } else { - p = get_kprobe(addr); if ((kcb->kprobe_status == KPROBE_HIT_SS) || (kcb->kprobe_status == KPROBE_REENTER)) { if (post_kprobe_handler(args->regs)) -- 2.47.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() 2025-05-17 9:30 [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() Mike Rapoport @ 2025-05-19 8:48 ` Geert Uytterhoeven 2025-05-19 10:01 ` Mike Rapoport 2025-06-07 13:10 ` John Paul Adrian Glaubitz 1 sibling, 1 reply; 5+ messages in thread From: Geert Uytterhoeven @ 2025-05-19 8:48 UTC (permalink / raw) To: Mike Rapoport Cc: John Paul Adrian Glaubitz, Mike Rapoport, Rich Felker, Yoshinori Sato, linux-kernel, linux-sh, kernel test robot Hi Mike, On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote: > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org> > > kbuild reports the following warning: > > arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify': > >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable] > 412 | struct kprobe *p = NULL; > | ^ > > The variable 'p' is indeed unused since the commit fa5a24b16f94 > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > Remove that variable along with 'kprobe_opcode_t *addr' which also > becomes unused after 'p' is removed. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/ > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org> Thanks for your patch! "p" and "addr" are definitely unused (besides side-effects?), so Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/arch/sh/kernel/kprobes.c > +++ b/arch/sh/kernel/kprobes.c > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) > int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data) > { > - struct kprobe *p = NULL; > struct die_args *args = (struct die_args *)data; > int ret = NOTIFY_DONE; > - kprobe_opcode_t *addr = NULL; > struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); > > - addr = (kprobe_opcode_t *) (args->regs->pc); > if (val == DIE_TRAP && > args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) { > if (!kprobe_running()) { > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > ret = NOTIFY_DONE; > } > } else { > - p = get_kprobe(addr); > if ((kcb->kprobe_status == KPROBE_HIT_SS) || > (kcb->kprobe_status == KPROBE_REENTER)) { > if (post_kprobe_handler(args->regs)) I have no idea what this code is supposed to do, and if it actually works. Red flags are that the assigned "p" was never used at all since the inception of this function. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() 2025-05-19 8:48 ` Geert Uytterhoeven @ 2025-05-19 10:01 ` Mike Rapoport 2025-05-19 10:39 ` Geert Uytterhoeven 0 siblings, 1 reply; 5+ messages in thread From: Mike Rapoport @ 2025-05-19 10:01 UTC (permalink / raw) To: Geert Uytterhoeven Cc: John Paul Adrian Glaubitz, Mike Rapoport, Rich Felker, Yoshinori Sato, linux-kernel, linux-sh, kernel test robot, Masami Hiramatsu On Mon, May 19, 2025 at 10:48:20AM +0200, Geert Uytterhoeven wrote: > Hi Mike, > > On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote: > > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org> > > > > kbuild reports the following warning: > > > > arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify': > > >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable] > > 412 | struct kprobe *p = NULL; > > | ^ > > > > The variable 'p' is indeed unused since the commit fa5a24b16f94 > > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > > > Remove that variable along with 'kprobe_opcode_t *addr' which also > > becomes unused after 'p' is removed. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/ > > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org> > > Thanks for your patch! > > "p" and "addr" are definitely unused (besides side-effects?), so > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/arch/sh/kernel/kprobes.c > > +++ b/arch/sh/kernel/kprobes.c > > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) > > int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > > unsigned long val, void *data) > > { > > - struct kprobe *p = NULL; > > struct die_args *args = (struct die_args *)data; > > int ret = NOTIFY_DONE; > > - kprobe_opcode_t *addr = NULL; > > struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); > > > > - addr = (kprobe_opcode_t *) (args->regs->pc); > > if (val == DIE_TRAP && > > args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) { > > if (!kprobe_running()) { > > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > > ret = NOTIFY_DONE; > > } > > } else { > > - p = get_kprobe(addr); > > if ((kcb->kprobe_status == KPROBE_HIT_SS) || > > (kcb->kprobe_status == KPROBE_REENTER)) { > > if (post_kprobe_handler(args->regs)) > > I have no idea what this code is supposed to do, and if it actually > works. Red flags are that the assigned "p" was never used at all > since the inception of this function. "p" was used before fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code"), but I can't say I understand that code either :) CC'ing Masami, he probably knows what this code does. > Gr{oetje,eeting}s, > > Geert -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() 2025-05-19 10:01 ` Mike Rapoport @ 2025-05-19 10:39 ` Geert Uytterhoeven 0 siblings, 0 replies; 5+ messages in thread From: Geert Uytterhoeven @ 2025-05-19 10:39 UTC (permalink / raw) To: Mike Rapoport Cc: John Paul Adrian Glaubitz, Mike Rapoport, Rich Felker, Yoshinori Sato, linux-kernel, linux-sh, kernel test robot, Masami Hiramatsu Hi Mike, On Mon, 19 May 2025 at 12:02, Mike Rapoport <rppt@kernel.org> wrote: > On Mon, May 19, 2025 at 10:48:20AM +0200, Geert Uytterhoeven wrote: > > On Sat, 17 May 2025 at 11:30, Mike Rapoport <rppt@kernel.org> wrote: > > > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org> > > > > > > kbuild reports the following warning: > > > > > > arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify': > > > >> arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable] > > > 412 | struct kprobe *p = NULL; > > > | ^ > > > > > > The variable 'p' is indeed unused since the commit fa5a24b16f94 > > > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > > > > > Remove that variable along with 'kprobe_opcode_t *addr' which also > > > becomes unused after 'p' is removed. > > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/ > > > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org> > > > > Thanks for your patch! > > > > "p" and "addr" are definitely unused (besides side-effects?), so > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > --- a/arch/sh/kernel/kprobes.c > > > +++ b/arch/sh/kernel/kprobes.c > > > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) > > > int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > > > unsigned long val, void *data) > > > { > > > - struct kprobe *p = NULL; > > > struct die_args *args = (struct die_args *)data; > > > int ret = NOTIFY_DONE; > > > - kprobe_opcode_t *addr = NULL; > > > struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); > > > > > > - addr = (kprobe_opcode_t *) (args->regs->pc); > > > if (val == DIE_TRAP && > > > args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) { > > > if (!kprobe_running()) { > > > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > > > ret = NOTIFY_DONE; > > > } > > > } else { > > > - p = get_kprobe(addr); > > > if ((kcb->kprobe_status == KPROBE_HIT_SS) || > > > (kcb->kprobe_status == KPROBE_REENTER)) { > > > if (post_kprobe_handler(args->regs)) > > > > I have no idea what this code is supposed to do, and if it actually > > works. Red flags are that the assigned "p" was never used at all > > since the inception of this function. > > "p" was used before fa5a24b16f94 ("sh/kprobes: Don't call the > ->break_handler() in SH kprobes code"), but I can't say I understand that > code either :) Yes, the _variable_ "p" was used before, but not before assigning a different value to it first. I guess this is where Rust would be helpful? Although C can and does give a warning here, too... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() 2025-05-17 9:30 [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() Mike Rapoport 2025-05-19 8:48 ` Geert Uytterhoeven @ 2025-06-07 13:10 ` John Paul Adrian Glaubitz 1 sibling, 0 replies; 5+ messages in thread From: John Paul Adrian Glaubitz @ 2025-06-07 13:10 UTC (permalink / raw) To: Mike Rapoport Cc: Mike Rapoport, Rich Felker, Yoshinori Sato, linux-kernel, linux-sh, kernel test robot Hi Mike, On Sat, 2025-05-17 at 12:30 +0300, Mike Rapoport wrote: > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org> > > kbuild reports the following warning: > > arch/sh/kernel/kprobes.c: In function 'kprobe_exceptions_notify': > > > arch/sh/kernel/kprobes.c:412:24: warning: variable 'p' set but not used [-Wunused-but-set-variable] > 412 | struct kprobe *p = NULL; > | ^ > > The variable 'p' is indeed unused since the commit fa5a24b16f94 > ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > > Remove that variable along with 'kprobe_opcode_t *addr' which also > becomes unused after 'p' is removed. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202505151341.EuRFR22l-lkp@intel.com/ > Fixes: fa5a24b16f94 ("sh/kprobes: Don't call the ->break_handler() in SH kprobes code") > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org> > --- > > I don't know why the warning poped up only now, the code there didn't > change for some time :/ > > arch/sh/kernel/kprobes.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/sh/kernel/kprobes.c b/arch/sh/kernel/kprobes.c > index 49c4ffd782d6..a250fb1b9420 100644 > --- a/arch/sh/kernel/kprobes.c > +++ b/arch/sh/kernel/kprobes.c > @@ -404,13 +404,10 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr) > int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data) > { > - struct kprobe *p = NULL; > struct die_args *args = (struct die_args *)data; > int ret = NOTIFY_DONE; > - kprobe_opcode_t *addr = NULL; > struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); > > - addr = (kprobe_opcode_t *) (args->regs->pc); > if (val == DIE_TRAP && > args->trapnr == (BREAKPOINT_INSTRUCTION & 0xff)) { > if (!kprobe_running()) { > @@ -421,7 +418,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > ret = NOTIFY_DONE; > } > } else { > - p = get_kprobe(addr); > if ((kcb->kprobe_status == KPROBE_HIT_SS) || > (kcb->kprobe_status == KPROBE_REENTER)) { > if (post_kprobe_handler(args->regs)) Thanks for catching this! Looks good to me! Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-07 13:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-17 9:30 [PATCH] sh: kprobes: remove unused variables in kprobe_exceptions_notify() Mike Rapoport 2025-05-19 8:48 ` Geert Uytterhoeven 2025-05-19 10:01 ` Mike Rapoport 2025-05-19 10:39 ` Geert Uytterhoeven 2025-06-07 13:10 ` John Paul Adrian Glaubitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox