Re: [PATCH V3] x86 spinlock: Fix memory corruption on completing completions

[Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>]

On 02/12/2015 08:30 PM, Peter Zijlstra wrote:
> On Thu, Feb 12, 2015 at 05:17:27PM +0530, Raghavendra K T wrote:
[...]
>>
>> Linus suggested that we should not do any writes to lock after unlock(),
>> and we can move slowpath clearing to fastpath lock.
>>
>> So this patch implements the fix with:
>> 1. Moving slowpath flag to head (Oleg).
>> 2. use xadd to avoid read/write after unlock that checks the need for
>> unlock_kick (Linus).
>
> Maybe spend a few more words explaining these things; something like:
>
> Unlocked locks don't care about the slowpath flag; therefore we can keep
> it set after the last unlock, as long as we clear it again on the first
> (try)lock -- this removes the write after unlock.

Nit: I 'll reword a bit here since slowpath flag would result in
unnecessary kick but otherwise harmless IMO.
something like:
Unlocked locks don't care about the slowpath flag; therefore we can keep
it set after the last unlock, and clear it again on the first (try)lock.
-- this removes the write after unlock. note that keeping slowpath flag 
would result in unnecessary kicks.

> By moving the slowpath flag from the tail to the head ticket we avoid
> the need to access both the head and tail tickets on unlock.
>
> We can further avoid the need for a read-after-release by using xadd;
> the prev head value will include the slowpath flag and indicate if we
> need to do PV kicking of suspended spinners -- on modern chips xadd
> isn't (much) more expensive than an add + load.
>
> Its 'obvious' now, but maybe not so much after we've all not looked at
> this for a few months.
>

Absolutely correct. Thanks Peter for the detailed and very helpful
writeup.