Re: [PATCH v3 1/7] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW_VAL

[Waiman Long]

On 07/26/2015 09:46 PM, Davidlohr Bueso wrote:
> On Sat, 2015-07-25 at 15:31 -0700, Davidlohr Bueso wrote:
> > On Wed, 2015-07-22 at 16:12 -0400, Waiman Long wrote:
> > > The smp_store_release() is not a full barrier. In order to avoid missed
> > > wakeup, we may need to add memory barrier around locked and cpu state
> > > variables adding to complexity. As the chance of spurious wakeup is very
> > > low, it is easier and safer to just do an unconditional kick at unlock
> > > time.
> > Although I guess if SPIN_THRESHOLD is ever enlarged, the chances of
> > spurious wakeups would be greater.
> >
> > > Signed-off-by: Waiman Long<Waiman.Long@xxxxxx>
> > Reviewed-by: Davidlohr Bueso<dave@xxxxxxxxxxxx>
> Thinking about this some more, as good practice, could you please add a
> comment in the code explicitly mentioning the spurious wakeup side
> effect? Perhaps even having something more generic for the entire kernel
> might be added/created to Documentation/spurious-wakeups.txt?
>
> Thanks,
> Davidlohr

The if conditional check was added with the intention to save an 
unneeded PV kick when the vCPU was running. Doing an unconditional kick 
doesn't do any harm other than the additional latency of the PV kick. I 
will add a comment when I update the patch.

As for the spurious-wakeups.txt, I saw there are spurious wakeup 
occasionally. However, I am not totally sure of the mechanism that 
causes it. Also the spurious wakeup here refers to the vmenter of the 
vCPU which is different from spurious wakeup of a sleeping thread. I 
don't think I have enough data and information to write a document file yet.

Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/