Messages in this thread | | | Date | Tue, 1 May 2018 07:36:01 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU |
| |
On Tue, May 01, 2018 at 10:24:01AM -0400, Steven Rostedt wrote: > On Mon, 30 Apr 2018 18:42:03 -0700 > Joel Fernandes <joelaf@google.com> wrote: > > > In recent tests with IRQ on/off tracepoints, a large performance > > overhead ~10% is noticed when running hackbench. This is root caused to > > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the > > tracepoint code. Following a long discussion on the list [1] about this, > > we concluded that srcu is a better alternative for use during rcu idle. > > Although it does involve extra barriers, its lighter than the sched-rcu > > version which has to do additional RCU calls to notify RCU idle about > > entry into RCU sections. > > > > In this patch, we change the underlying implementation of the > > trace_*_rcuidle API to use SRCU. This has shown to improve performance > > alot for the high frequency irq enable/disable tracepoints.
[ . . . ]
> > --- a/kernel/tracepoint.c > > +++ b/kernel/tracepoint.c > > @@ -31,6 +31,9 @@ > > extern struct tracepoint * const __start___tracepoints_ptrs[]; > > extern struct tracepoint * const __stop___tracepoints_ptrs[]; > > > > +DEFINE_SRCU(tracepoint_srcu); > > +EXPORT_SYMBOL_GPL(tracepoint_srcu); > > + > > /* Set to 1 to enable tracepoint debug output */ > > static const int tracepoint_debug; > > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count) > > return p == NULL ? NULL : p->probes; > > } > > > > -static void rcu_free_old_probes(struct rcu_head *head) > > +static void srcu_free_old_probes(struct rcu_head *head) > > { > > kfree(container_of(head, struct tp_probes, rcu)); > > } > > > > +static void rcu_free_old_probes(struct rcu_head *head) > > +{ > > + call_srcu(&tracepoint_srcu, head, srcu_free_old_probes); > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I guess > it would be.
It is perfectly legal, and quite a bit simpler than setting something up to wait for both to complete concurrently.
Of course, if you unconditionally call call_srcu() from that same srcu_struct's callback, SRCU will be unable to safely delete the srcu_struct, so cleanup_srcu_struct() will react by leaking memory. ;-)
Normal RCU deals with the analogous situation by leaving at least one callback uninvoked when the system goes down.
Thanx, Paul
> I think we should add a comment to why we are doing this. Something > like: > > /* > * Tracepoint probes are protected by both sched RCU and SRCU, by > * calling the SRCU callback in the sched RCU callback we cover > * both cases. > */ > > Or something along those lines. > > -- Steve > > > > +} > > + > > static inline void release_probes(struct tracepoint_func *old) > > { > > if (old) { >
| |