Messages in this thread |  | | Date | Sat, 10 Dec 2022 09:47:53 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [for-next][PATCH 13/25] x86/mm/kmmio: Use rcu_read_lock_sched_notrace() |
| |
On Sat, Dec 10, 2022 at 08:58:03AM -0500, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The mmiotrace tracer is "special". The purpose is to help reverse engineer > binary drivers by removing the memory allocated by the driver and when the > driver goes to access it, a fault occurs, the mmiotracer will record what > the driver was doing and then do the work on its behalf by single stepping > through the process. > > But to achieve this ability, it must do some special things. One is to > take the rcu_read_lock() when the fault occurs, and then release it in the > breakpoint that is single stepping. This makes lockdep unhappy, as it > changes the state of RCU from within an exception that is not contained in > that exception, and we get a nasty splat from lockdep. > > Instead, switch to rcu_read_lock_sched_notrace() as the RCU sched variant > has the same grace period as normal RCU. This is basically the same as > rcu_read_lock() but does not make lockdep complain about it. > > Note, the preempt_disable() is still needed as it uses preempt_enable_no_resched(). > > Link: https://lore.kernel.org/linux-trace-kernel/20221209134144.04f33626@gandalf.local.home > > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Karol Herbst <karolherbst@gmail.com> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Executable code can be the best form of comment. ;-)
This does mess with preempt_count() redundantly, but the overhead from that should be way down in the noise.
Acked-by: Paul E. McKenney <paulmck@kernel.org>
> --- > arch/x86/mm/kmmio.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/mm/kmmio.c b/arch/x86/mm/kmmio.c > index edb486450158..853c49877c16 100644 > --- a/arch/x86/mm/kmmio.c > +++ b/arch/x86/mm/kmmio.c > @@ -254,7 +254,7 @@ int kmmio_handler(struct pt_regs *regs, unsigned long addr) > * again. > */ > preempt_disable(); > - rcu_read_lock(); > + rcu_read_lock_sched_notrace(); > > faultpage = get_kmmio_fault_page(page_base); > if (!faultpage) { > @@ -323,7 +323,7 @@ int kmmio_handler(struct pt_regs *regs, unsigned long addr) > return 1; /* fault handled */ > > no_kmmio: > - rcu_read_unlock(); > + rcu_read_unlock_sched_notrace(); > preempt_enable_no_resched(); > return ret; > } > @@ -363,7 +363,7 @@ static int post_kmmio_handler(unsigned long condition, struct pt_regs *regs) > /* These were acquired in kmmio_handler(). */ > ctx->active--; > BUG_ON(ctx->active); > - rcu_read_unlock(); > + rcu_read_unlock_sched_notrace(); > preempt_enable_no_resched(); > > /* > -- > 2.35.1 > >
|  |