On Sat, Aug 09, 2014 at 06:38:29PM -0700, Paul E. McKenney wrote: > On Sat, Aug 09, 2014 at 08:33:55PM +0200, Peter Zijlstra wrote: > > 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. > > You want me not to disable irqs before invoking rcu_preempt_qs() from > rcu_preempt_note_context_switch(), I get that. But right now, they > really are disabled courtesy of the local_irq_save() before the call > to rcu_preempt_qs() from rcu_preempt_note_context_switch().
Ah, confusion there, I said rcu_sched_qs(), you're talking about
rcu_preempt_qs().
Yes the call to rcu_preempt_qs() is unconditionally wrapped in IRQ
disable.
> > 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.
>
> It might be better to avoid storing to rcu_sched_data.passed_quiesce when
> it is already 1, though the difference would be quite hard to measure.
> In that case, this would work nicely:
>
> static void rcu_preempt_qs(int cpu)
> {
> if (rdp->passed_quiesce == 0) {
> trace_rcu_grace_period(TPS("rcu_preempt"), rdp->gpnum,
> TPS("cpuqs"));
> > __this_cpu_write(rcu_sched_data.passed_quiesce, 1);
> }
> current->rcu_read_unlock_special &= ~RCU_READ_UNLOCK_NEED_QS;
> }
Yes, that's a consideration, fair enough. Again note the confusion
between sched/preempt. But yes, both can use this 'cleanup'.
pgpNaP_6DAk0N.pgp
Description: PGP signature
