Re: [Xen-devel] [PATCHv3 3/4] xen: use ticket locks for spin locks

[Tim Deegan <tim@xxxxxxx>]

At 14:45 +0100 on 23 Apr (1429800338), Andrew Cooper wrote:
> On 23/04/15 14:43, David Vrabel wrote:
> > On 23/04/15 13:03, Tim Deegan wrote:
> >> Hi,
> >>
> >> At 11:11 +0100 on 21 Apr (1429614687), David Vrabel wrote:
> >>>  void _spin_lock(spinlock_t *lock)
> >>>  {
> >>> +    spinlock_tickets_t tickets = { .tail = 1, };
> >>>      LOCK_PROFILE_VAR;
> >>>  
> >>>      check_lock(&lock->debug);
> >>> -    while ( unlikely(!_raw_spin_trylock(&lock->raw)) )
> >>> +    tickets.head_tail = xadd(&lock->tickets.head_tail, 
> >>> tickets.head_tail);
> >>> +    while ( tickets.tail != observe_head(&lock->tickets) )
> >>>      {
> >>>          LOCK_PROFILE_BLOCK;
> >>> -        while ( likely(_raw_spin_is_locked(&lock->raw)) )
> >>> -            cpu_relax();
> >>> +        cpu_relax();
> >>>      }
> >>>      LOCK_PROFILE_GOT;
> >>>      preempt_disable();
> >> I think you need an smp_mb() here to make sure that locked accesses
> >> don't get hoisted past the wait-for-my-ticket loop by an out-of-order
> >> (ARM) cpu.
> > Ok, but smp_mb() turns into an mfence on x86.  Is this a
> > problem/sub-optimal?
> 
> That can probably change back to a plain compiler barrier as Xen only
> supports 64bit and doesn't need PPRO ordering errata workarounds.

And for the archive: smp_mb() needs to be an mfence in the general
case, otherwise "WRITE; smp_mb(); READ;" can be reordered, if the write
and read are to different addresses.

Tim.

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