linux-kernel - Re: [PATCH v3 1/7] locking/pvqspinlock: Unconditional PV kick with _Q_SLOW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 Jul 2015 13:50:59 -0400
From:	Waiman Long <waiman.long@...com>
To:	Davidlohr Bueso <dave@...olabs.net>
CC:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-kernel@...r.kernel.org, Scott J Norton <scott.norton@...com>,
	Douglas Hatch <doug.hatch@...com>
Subject: Re: [PATCH v3 1/7] locking/pvqspinlock: Unconditional PV kick with
 _Q_SLOW_VAL

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@...com>
>> Reviewed-by: Davidlohr Bueso<dave@...olabs.net>
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/