| Message ID | 1043d06a0affed83a4a46dd29466e72820ee215d.1666262278.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2fa9482334b0593b7edc371a13c0cca81daaa89e |
| Headers | show |
| Series | powerpc/kprobes: preempt related changes and cleanups | expand |
On Fri Oct 21, 2022 at 3:28 AM AEST, Naveen N. Rao wrote: > arch_prepare_kprobe() is called from register_kprobe() via > prepare_kprobe(), or through register_aggr_kprobe(), both with the > kprobe_mutex held. Per the comment for get_kprobe(): > /* > * This routine is called either: > * - under the 'kprobe_mutex' - during kprobe_[un]register(). > * OR > * - with preemption disabled - from architecture specific code. > */ That comment should read [un]register_kprobe(), right? > > As such, there is no need to disable preemption around the call to > get_kprobe(). Drop the same. And prepare_kprobe() and register_aggr_kprobe() are both called with kprobe_mutex held so you rely on the same protection. This seems to fix a lost-resched bug with preempt kernels too. Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > > Reported-by: Nicholas Piggin <npiggin@gmail.com> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/kprobes.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c > index bd7b1a03545948..88f42de681e1f8 100644 > --- a/arch/powerpc/kernel/kprobes.c > +++ b/arch/powerpc/kernel/kprobes.c > @@ -158,9 +158,7 @@ int arch_prepare_kprobe(struct kprobe *p) > printk("Cannot register a kprobe on the second word of prefixed instruction\n"); > ret = -EINVAL; > } > - preempt_disable(); > prev = get_kprobe(p->addr - 1); > - preempt_enable_no_resched(); > > /* > * When prev is a ftrace-based kprobe, we don't have an insn, and it > -- > 2.38.0
Nicholas Piggin wrote: > On Fri Oct 21, 2022 at 3:28 AM AEST, Naveen N. Rao wrote: >> arch_prepare_kprobe() is called from register_kprobe() via >> prepare_kprobe(), or through register_aggr_kprobe(), both with the >> kprobe_mutex held. Per the comment for get_kprobe(): >> /* >> * This routine is called either: >> * - under the 'kprobe_mutex' - during kprobe_[un]register(). >> * OR >> * - with preemption disabled - from architecture specific code. >> */ > > That comment should read [un]register_kprobe(), right? Ugh, yes! > >> >> As such, there is no need to disable preemption around the call to >> get_kprobe(). Drop the same. > > And prepare_kprobe() and register_aggr_kprobe() are both called with > kprobe_mutex held so you rely on the same protection. This seems to > fix a lost-resched bug with preempt kernels too. > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com> Thanks, Naveen
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index bd7b1a03545948..88f42de681e1f8 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -158,9 +158,7 @@ int arch_prepare_kprobe(struct kprobe *p) printk("Cannot register a kprobe on the second word of prefixed instruction\n"); ret = -EINVAL; } - preempt_disable(); prev = get_kprobe(p->addr - 1); - preempt_enable_no_resched(); /* * When prev is a ftrace-based kprobe, we don't have an insn, and it
arch_prepare_kprobe() is called from register_kprobe() via prepare_kprobe(), or through register_aggr_kprobe(), both with the kprobe_mutex held. Per the comment for get_kprobe(): /* * This routine is called either: * - under the 'kprobe_mutex' - during kprobe_[un]register(). * OR * - with preemption disabled - from architecture specific code. */ As such, there is no need to disable preemption around the call to get_kprobe(). Drop the same. Reported-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- arch/powerpc/kernel/kprobes.c | 2 -- 1 file changed, 2 deletions(-)