linux-kernel - Re: [PATCH] pv-qspinlock: Try to re-hash the lock after spurious

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:	Fri, 27 May 2016 23:41:28 -0400
From:	Waiman Long <waiman.long@....com>
To:	xinhui <xinhui.pan@...ux.vnet.ibm.com>
CC:	<linux-kernel@...r.kernel.org>, <peterz@...radead.org>,
	<mingo@...hat.com>
Subject: Re: [PATCH] pv-qspinlock: Try to re-hash the lock after spurious_wakeup

On 05/27/2016 06:32 AM, xinhui wrote:
>
> On 2016年05月27日 02:31, Waiman Long wrote:
>> On 05/25/2016 02:09 AM, Pan Xinhui wrote:
>>> In pv_wait_head_or_lock, if there is a spurious_wakeup, and it fails to
>>> get the lock as there is lock stealing, then after a short spin, we 
>>> need
>>> hash the lock again and enter pv_wait to yield.
>>>
>>> Currently after a spurious_wakeup, as l->locked is not _Q_SLOW_VAL,
>>> pv_wait might do nothing and return directly, that is not
>>> paravirt-friendly because pv_wait_head_or_lock will just spin on the
>>> lock then.
>>>
>>> Signed-off-by: Pan Xinhui<xinhui.pan@...ux.vnet.ibm.com>
>>> ---
>>>   kernel/locking/qspinlock_paravirt.h | 39 
>>> +++++++++++++++++++++++++++++--------
>>>   1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> Is this a problem you can easily reproduce on PPC? I have not 
>> observed this issue when testing on x86.
>>
> Hi, Waiman
>     I notice the spurious_wakeup count is very high when I do 
> benchmark tests and stress tests. So after a simple investigation,
> I find pv_wait_head_or_lock() just keep loops.
>

That shouldn't happen in normal case. When testing on x86, I typically 
get the following stat data for an over-commited guest:

pv_lock_slowpath=9256211
pv_lock_stealing=36398363
pv_spurious_wakeup=311
pv_wait_again=294
pv_wait_early=3255605
pv_wait_head=173
pv_wait_node=3256280

The queue head don't call pv_wait that often. There are a bit of 
spurious wakeup, but it is mostly caused by lock stealing. How long is a 
cpu_relax() in PPC takes?

>     Here is my story, in my pv-qspinlcok patchset V1&&v2, pv_wait on 
> ppc ignore the first two parameters of *ptr and val, that makes 
> lock_stealing hit too much.

The pvqspinlock code does depend on pv_wait() doing a final check to see 
if the lock value change. The code may not work reliably without that.

> and when I change SPIN_THRESHOLD to a small value, system is very much 
> unstable because waiter will enter pv_wait quickly and no one will 
> kick waiter's cpu if
> we enter pv_wait twice thanks to the lock_stealing.
>     So what I do in my pv-qspinlcok patchset V3 is that add if (*ptr 
> == val) in pv_wait. However as I mentioned above, then spurious_wakeup 
> count is too high, that also means our cpu
> slice is wasted.

The SPIN_THRESHOLD should be sufficiently big. A small value will cause 
too many waits and wake-up's which may not be good. Anyway, more testing 
and tuning may be needed to make the pvqspinlock code work well with PPC.

Cheers,
Longman