Re: [Xen-devel] [PATCHv4 3/5] xen: use ticket locks for spin locks

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Tim Deegan <tim@xxxxxxx> 05/14/15 10:55 PM >>>
>At 21:05 +0100 on 14 May (1431637535), Jan Beulich wrote:
>> >>> Tim Deegan <tim@xxxxxxx> 05/14/15 12:36 PM >>>
>> >At 15:37 +0100 on 11 May (1431358623), David Vrabel wrote:
>> >> +        while ( observe_head(&lock->tickets) != sample.tail )
>> >
>> >This test should be "observe_head(&lock->tickets) == sample.head",
>> >i.e. wait until the thread that held the lock has released it.
>> >Checking for it to reach the tail is unnecessary (other threads that
>> >were queueing for the lock at the sample time don't matter) and
>> >dangerous (on a contended lock head might pass sample.tail without us
>> >happening to observe it being == ).
>> 
>> The observation of there being a problem is correct, but the suggested 
>> solution
>> doesn't seem to be. The new code being
>> 
>>     if ( sample.head != sample.tail )
>>     {
>>         while ( observe_head(&lock->tickets) == sample.tail )
>>             cpu_relax();
>> 
>> means that if head didn't change between the full sample and the head sample
>> we'd end the loop right away, which can't be right. We really need to wait 
>> for
>> head to reach or pass the sampled tail.
>
>I think you misread what I asked for.  We wait until the observed head
>doesn't match the sampled _head_, i.e. for whoever had the lock when
>we sampled it to realease it:
>
>if ( sample.head != sample.tail )
>{
>while ( observe_head(&lock->tickets) == sample.head )
>cpu_relax();

Indeed. And in my simultaneously written reply to that mail I (once again)
mixed up head and tail. Sorry for the noise then.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel