Messages in this thread | | | Date | Fri, 27 Jul 2018 11:51:13 -0700 | From | Joel Fernandes <> | Subject | Re: [PATCH v11 2/3] tracepoint: Make rcuidle tracepoint callers use SRCU |
| |
On Fri, Jul 27, 2018 at 12:26:59PM -0400, Steven Rostedt wrote: > On Thu, 26 Jul 2018 16:50:43 -0700 > Joel Fernandes <joel@joelfernandes.org> wrote: > > > F > > include/linux/tracepoint.h | 41 ++++++++++++++++++++++++++++++-------- > > kernel/tracepoint.c | 16 ++++++++++++++- > > 2 files changed, 48 insertions(+), 9 deletions(-) > > > > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h > > index 19a690b559ca..6e7bc6ebfcd8 100644 > > --- a/include/linux/tracepoint.h > > +++ b/include/linux/tracepoint.h > > @@ -15,6 +15,7 @@ > > */ > > > > #include <linux/smp.h> > > +#include <linux/srcu.h> > > #include <linux/errno.h> > > #include <linux/types.h> > > #include <linux/cpumask.h> > > @@ -33,6 +34,8 @@ struct trace_eval_map { > > > > #define TRACEPOINT_DEFAULT_PRIO 10 > > > > +extern struct srcu_struct tracepoint_srcu; > > + > > extern int > > tracepoint_probe_register(struct tracepoint *tp, void *probe, void *data); > > extern int > > @@ -75,10 +78,16 @@ int unregister_tracepoint_module_notifier(struct notifier_block *nb) > > * probe unregistration and the end of module exit to make sure there is no > > * caller executing a probe when it is freed. > > */ > > +#ifdef CONFIG_TRACEPOINTS > > static inline void tracepoint_synchronize_unregister(void) > > { > > + synchronize_srcu(&tracepoint_srcu); > > synchronize_sched(); > > } > > +#else > > +static inline void tracepoint_synchronize_unregister(void) > > +{ } > > +#endif > > > > #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS > > extern int syscall_regfunc(void); > > @@ -129,18 +138,32 @@ extern void syscall_unregfunc(void); > > * as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just > > * "void *data", where as the DECLARE_TRACE() will pass in "void *data, proto". > > */ > > -#define __DO_TRACE(tp, proto, args, cond, rcucheck) \ > > +#define __DO_TRACE(tp, proto, args, cond, rcuidle) \ > > do { \ > > struct tracepoint_func *it_func_ptr; \ > > void *it_func; \ > > void *__data; \ > > + int __maybe_unused idx = 0; \ > > \ > > if (!(cond)) \ > > return; \ > > - if (rcucheck) \ > > - rcu_irq_enter_irqson(); \ > > - rcu_read_lock_sched_notrace(); \ > > - it_func_ptr = rcu_dereference_sched((tp)->funcs); \ > > + \ > > + /* srcu can't be used from NMI */ \ > > + if (rcuidle && in_nmi()) \ > > + WARN_ON_ONCE(1); \ > > Why isn't the above: > > WARN_ON_ONCE(rcuidle && in_nmi()); > > ? > > The rest looks fine.
When I refactored the code as Peter suggested I happen to move the warning out but didn't simplify it further (I had it at the back of my head to do that but missed it).
Since its quite a simple change I can send a follow up patch doing so later, or can resend this one. What do you suggest?
thanks,
- Joel
| |