Re: [Xen-devel] [PATCH 03/11] qspinlock: Add pending bit

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Jun 18, 2014 at 01:29:48PM +0200, Paolo Bonzini wrote:
> Il 17/06/2014 22:36, Konrad Rzeszutek Wilk ha scritto:
> >+    /* One more attempt - but if we fail mark it as pending. */
> >+    if (val == _Q_LOCKED_VAL) {
> >+            new = Q_LOCKED_VAL |_Q_PENDING_VAL;
> >+
> >+            old = atomic_cmpxchg(&lock->val, val, new);
> >+            if (old == _Q_LOCKED_VAL) /* YEEY! */
> >+                    return;
> >+            val = old;
> >+    }
> 
> Note that Peter's code is in a for(;;) loop:
> 
> 
> +     for (;;) {
> +             /*
> +              * If we observe any contention; queue.
> +              */
> +             if (val & ~_Q_LOCKED_MASK)
> +                     goto queue;
> +
> +             new = _Q_LOCKED_VAL;
> +             if (val == new)
> +                     new |= _Q_PENDING_VAL;
> +
> +             old = atomic_cmpxchg(&lock->val, val, new);
> +             if (old == val)
> +                     break;
> +
> +             val = old;
> +     }
> +
> +     /*
> +      * we won the trylock
> +      */
> +     if (new == _Q_LOCKED_VAL)
> +             return;
> 
> So what you'd have is basically:
> 
>       /*
>        * One more attempt if no one is already in queue.  Perhaps
>        * they have unlocked the spinlock already.
>        */
>       if (val == _Q_LOCKED_VAL && atomic_read(&lock->val) == 0) {
>               old = atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL);
>               if (old == 0) /* YEEY! */
>                       return;
>               val = old;
>       }
> 
> But I agree with Waiman that this is unlikely to trigger often enough. It
> does have to be handled in the slowpath for correctness, but the most likely
> path is (0,0,1)->(0,1,1).

<nods>
> 
> Paolo

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