Re: [Xen-devel] [PATCH 3/5] xen: RCU/x86/ARM: discount CPUs that were idle when grace period started.

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 11:19 +0200 on 14 Aug (1502709563), Dario Faggioli wrote:
> Basically, for the race to occur (in [3]), it is necessary that [2]
> (CPU1 doing rcp->cur++) happens after [1] (last call to rcu_pending()
> of CPU2, before clearing the mask and going idle). In fact, it that is
> not the case, rcu_pending() in CPU2 will say 'no', because of this
> check:
> 
>  if (rdp->quiescbatch != rcp->cur || rdp->qs_pending)
> 
> which would prevent the CPU from going idle (and will, at least, lead
> to it going through check_quiescent_state() twice, clearing itself from
> the new grace period too).
> 
> Therefore, it looks to me that the race can be avoided by making sure
> that there is at least one check of rcu_pending(), after a CPU has
> called rcu_enter_idle(). This basically means moving rcu_idle_enter()
> up.

Sounds plausible.

> I was actually thinking to move it inside the tick-stopping logic too.
> This way, either, when CPU1 samples the cpumask for the new grace
> period:
> 1) CPU2 will be in idle_cpumask already, and it actually goes idle;

The system works!

> 2) CPU2 will be in idle_cpumask, but then it does not go idle
> (cpu_is_haltable() returning false) for various reasons. This may look
> unideal, but if it is here, trying to go idle, it is not holding
> references to read-side critical sections, or at least we can consider
> it to be quiescing, so it's ok to ignore it for the grace period;

Yes.  This is equivalent to CPU2 actually going idle and then waking
up quickly.  As you say, since at this point the RCU logic thinks the
CPU can idle, whatever stopped it actually idling can't have been
relevant to RCU.

> 3) CPU2 is not in idle_cpumask, and so it will not be in the sampled
> mask, but the first check of rcu_pending() will lead acknowledge
> quiescence, and calling of cpu_quiet();

Yep.  And because it's not yet in idle_cpumask, it _will_ check
rcu_pending() before idling.  I think that needs an smp_mb() between
setting the idle_cpumask and checking rcp->cur, and likewise between
rcp->cur++ and cpumask_andnot() in rcu_start_batch().

Sounds good!

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel