Re: [RFC 0/3] Revert SRCU from tracepoint infrastructure

From: Steven Rostedt
Date: Mon Feb 10 2020 - 15:03:53 EST


On Mon, 10 Feb 2020 14:53:02 -0500
Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote:

> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index 1fb11daa5c53..a83fd076a312 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -179,10 +179,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > * For rcuidle callers, use srcu since sched-rcu \
> > * doesn't work from the idle path. \
> > */ \
> > - if (rcuidle) { \
> > + if (rcuidle) \
> > __idx = srcu_read_lock_notrace(&tracepoint_srcu);\
> > - rcu_irq_enter_irqson(); \
> > - } \
>
> This would still break out-of-tree modules or future code that does
> rcu_read_lock() right in a tracepoint callback right?

Yes, and that's fine.

>
> Or are we saying that rcu_read_lock() in a tracepoint callback is not
> allowed? I believe this should then at least be documented somewhere. Also,

No, it's only not allowed if you you attached to a tracepoint that can
be called without rcu watching. That's up to the caller to figure it
out. Tracepoints were never meant to be a generic thing people should
use without knowing what they are really doing.

> what about code in tracepoint callback that calls rcu_read_lock() indirectly
> through a path in the kernel, and also code that may expect RCU readers when
> doing preempt_disable()?

Then they need to know what they are doing.

>
> So basically we are saying with this patch:
> 1. Don't call in a callback: rcu_read_lock() or preempt_disable() and expect RCU to do
> anything for you.

We can just say, "If you plan on using RCU, be aware that it man not be
watching and you get do deal with the fallout. Use rcu_is_watching() to
figure it out."

> 2. Don't call code that does anything that 1. needs.
>
> Is that intended? thanks,
>

No, look what the patch did for perf. Why make *all* callbacks suffer
if only some use RCU? If you use RCU from a callback, then you need to
figure it out. The same goes for attaching to the function tracer.

-- Steve