Messages in this thread | | | Date | Thu, 12 Feb 2015 20:58:16 +0530 | From | Raghavendra K T <> | Subject | Re: [PATCH V3] x86 spinlock: Fix memory corruption on completing completions |
| |
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.
| |