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

[Tim Deegan <tim@xxxxxxx>]

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.

>  void _spin_unlock(spinlock_t *lock)
>  {
> +    smp_mb();
>      preempt_enable();
>      LOCK_PROFILE_REL;
> -    _raw_spin_unlock(&lock->raw);
> +    lock->tickets.head++;

This needs to be done with an explicit atomic (though not locked)
write; otherwise the compiler might use some unsuitable operation that
clobbers .tail as well.  (For some reason I convinced myself last
time that this was OK, but I can't remember why. :))

Cheers,

Tim.

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