linux-kernel - Re: [PATCH v7 4/5] locking/pvqspinlock: Allow 1 lock stealing attempt

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] [day] [month] [year] [list]

Message-ID: <56201443.70406@hpe.com>
Date:	Thu, 15 Oct 2015 17:01:55 -0400
From:	Waiman Long <waiman.long@....com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	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>,
	Davidlohr Bueso <dave@...olabs.net>
Subject: Re: [PATCH v7 4/5] locking/pvqspinlock: Allow 1 lock stealing attempt

On 10/14/2015 05:28 AM, Peter Zijlstra wrote:
> On Tue, Oct 13, 2015 at 04:50:25PM -0400, Waiman Long wrote:
>> On 10/13/2015 03:56 PM, Peter Zijlstra wrote:
>>> So the below is exactly duplicated from the normal slowpath, so why
>>> don't you keep that there?
>>>
>>> It would get you something like:
>>>
>>> 	if (pv_wait_head_or_steal(..))
>>> 		goto stolen;
>>>
>>>
>>> stolen:
>>>> +	/*
>>>> +	 * contended path; wait for next, release.
>>>> +	 */
>>>> +	while (!(next = READ_ONCE(node->next)))
>>>> +		cpu_relax();
>>>> +
>>>> +	arch_mcs_spin_unlock_contended(&next->locked);
>>>> +	pv_kick_node(lock, next);
>>> release:
>>> 	...
>> Yes, it is largely the same. I thought that you don't like too much change
>> in the logic flow of the generic qspinlock code. I will make the change in
>> the next revision.
> Well, you already put the branch in there, the only difference here is
> an 'extra' label. OTOH that extra label avoids duplicating some hairy
> code. So over all I would say its a definite win.
>
> And its easy to see it will compile away on the native case where:
>
>   #define pv_wait_head_or_steal(l, n, t) (false)
>
>

I have done it in a different way to avoid duplicating code. I think I 
am not going to use the pv_wait_head_or_steal name after all as I don't 
consider acquiring the lock by the queue head CPU is lock stealing. I 
will skip "and" and name it as pv_wait_head_lock instead. Please let me 
know if you have a better idea.

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/