Messages in this thread | | | Date | Sat, 9 Aug 2014 20:33:55 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3 tip/core/rcu 1/9] rcu: Add call_rcu_tasks() |
| |
On Fri, Aug 08, 2014 at 01:58:26PM -0700, Paul E. McKenney wrote: > > > And on that, you probably should change rcu_sched_rq() to read: > > > > this_cpu_inc(rcu_sched_data.passed_quiesce); > > > > That avoids touching the per-cpu data offset. > > Hmmm... Interrupts are disabled,
No they are not, __schedule()->rcu_note_context_switch()->rcu_sched_qs() is only called with preemption disabled.
We only disable IRQs later, where we take the rq->lock.
> so no need to further disable > interrupts. Storing 1 works fine, no need to increment. If I followed > the twisty per_cpu passages correctly, my guess is that you would like > me to do something like this: > > __this_cpu_write(rcu_sched_data.passed_quiesce, 1); > > Does that work?
Yeah, should be more or less similar, the inc might be encoded shorter due to not requiring an immediate, but who cares :-)
void rcu_sched_qs(int cpu) { if (trace_rcu_grace_period_enabled()) { if (!__this_cpu_read(rcu_sched_data.passed_quiesce)) trace_rcu_grace_period(...); } __this_cpu_write(rcu_sched_data.passed_quiesce, 1); }
Would further avoid emitting the conditional in the normal case where the tracepoint is inactive.
Steve does it make sense to have __DO_TRACE() emit __trace_##name() to avoid the double static_branch thing?
> > And it would be very good if we could avoid the unconditional IRQ flag > > fiddling in rcu_preempt_note_context_switch(), them expensive, this > > looks entirely feasibly in the 'normal' case where > > t->rcu_read_unlock_special doesn't have RCU_READ_UNLOCK_NEED_QS set. > > Agreed, but sometimes RCU_READ_UNLOCK_NEED_QS is set. > > That said, I should probably revisit RCU_READ_UNLOCK_NEED_QS. A lot has > changed since I wrote that code.
Sure, but a conditional testing RCU_READ_UNLOCK_NEED_QS is far cheaper than poking the IRQ flags. That said, its not entirely clear to me why that needs IRQs disabled at all, then again I didn't look long and I'm sure its all subtle. [unhandled content-type:application/pgp-signature] | |