[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> "Jan Beulich" <jbeulich@xxxxxxxx> 05/14/15 10:06 PM >>>
>>>> Tim Deegan <tim@xxxxxxx> 05/14/15 12:36 PM >>>
>>At 15:37 +0100 on 11 May (1431358623), David Vrabel wrote:
>>>  void _spin_barrier(spinlock_t *lock)
>>>  {
>>> +    spinlock_tickets_t sample;
>>>  #ifdef LOCK_PROFILE
>>>      s_time_t block = NOW();
>>> -    u64      loop = 0;
>>> +#endif
>>>  
>>>      check_barrier(&lock->debug);
>>> -    do { smp_mb(); loop++;} while ( _raw_spin_is_locked(&lock->raw) );
>>> -    if ((loop > 1) && lock->profile)
>>> +    smp_mb();
>>> +    sample = observe_lock(&lock->tickets);
>>> +    if ( sample.head != sample.tail )
>>>      {
>>> -        lock->profile->time_block += NOW() - block;
>>> -        lock->profile->block_cnt++;
>>> -    }
>>> -#else
>>> -    check_barrier(&lock->debug);
>>> -    do { smp_mb(); } while ( _raw_spin_is_locked(&lock->raw) );
>>> +        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.

Just realized that this would be too heavy again (and more difficult to do than
necessary) - I think on the left side we need observe_tail() instead of
observe_head(), and then the == is correct.

Jan

Jan


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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.