Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1534155

Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop

From Chris Wilson <chris@chris-wilson.co.uk>
Newsgroups linux.kernel
Subject Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop
Date 2016-12-01 15:40 +0100
Message-ID <sJAGK-53E-9@gated-at.bofh.it> (permalink)
References <sJAdI-4Rh-7@gated-at.bofh.it> <sJAdJ-4Rh-45@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Dec 01, 2016 at 03:06:45PM +0100, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <Nicolai.Haehnle@amd.com>
> 
> In the following scenario, thread #1 should back off its attempt to lock
> ww1 and unlock ww2 (assuming the acquire context stamps are ordered
> accordingly).
> 
>     Thread #0               Thread #1
>     ---------               ---------
>                             successfully lock ww2
>     set ww1->base.owner
>                             attempt to lock ww1
>                             confirm ww1->ctx == NULL
>                             enter mutex_spin_on_owner
>     set ww1->ctx
> 
> What was likely to happen previously is:
> 
>     attempt to lock ww2
>     refuse to spin because
>       ww2->ctx != NULL
>     schedule()
>                             detect thread #0 is off CPU
>                             stop optimistic spin
>                             return -EDEADLK
>                             unlock ww2
>                             wakeup thread #0
>     lock ww2
> 
> Now, we are more likely to see:
> 
>                             detect ww1->ctx != NULL
>                             stop optimistic spin
>                             return -EDEADLK
>                             unlock ww2
>     successfully lock ww2
> 
> ... because thread #1 will stop its optimistic spin as soon as possible.
> 
> The whole scenario is quite unlikely, since it requires thread #1 to get
> between thread #0 setting the owner and setting the ctx. But since we're
> idling here anyway, the additional check is basically free.
> 
> Found by inspection.

Similar question can be raised for can_spin_on_owner() as well. Is it
worth for a contending ww_mutex to enter the osq queue if we expect a
EDEADLK? It seems to boil down to how likely is the EDEADLK going to
evaporate if we wait for the owner to finish and unlock.

The patch looks reasonable, just a question of desirability.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 00/11] locking/ww_mutex: Keep sorted wait list to avoid stampedes Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
  [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
    Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 17:10 +0100
    Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Peter Zijlstra <peterz@infradead.org> - 2016-12-06 16:40 +0100
    Re: [PATCH v2 05/11] locking/ww_mutex: Add waiters in stamp order Peter Zijlstra <peterz@infradead.org> - 2016-12-06 18:00 +0100
  [PATCH v2 07/11] locking/ww_mutex: Wake at most one waiter for back off when acquiring the lock Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
  [PATCH v2 10/11] Documentation/locking/ww_mutex: Update the design document Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
  [PATCH v2 09/11] locking/mutex: Initialize mutex_waiter::ww_ctx with poison when debugging Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
  [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner optimistic spin loop Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
    Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner  optimistic spin loop Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 15:40 +0100
    Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner  optimistic spin loop Peter Zijlstra <peterz@infradead.org> - 2016-12-06 16:10 +0100
      Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner  optimistic spin loop Waiman Long <longman@redhat.com> - 2016-12-06 17:10 +0100
        Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner  optimistic spin loop Waiman Long <longman@redhat.com> - 2016-12-06 19:50 +0100
        Re: [PATCH v2 02/11] locking/ww_mutex: Re-check ww->ctx in the inner  optimistic spin loop Peter Zijlstra <peterz@infradead.org> - 2016-12-06 20:10 +0100
  [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a NULL context Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
    Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a  NULL context Chris Wilson <chris@chris-wilson.co.uk> - 2016-12-01 15:20 +0100
      Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a  NULL context Daniel Vetter <daniel@ffwll.ch> - 2016-12-01 16:20 +0100
    Re: [PATCH v2 01/11] drm/vgem: Use ww_mutex_(un)lock even with a  NULL context Peter Zijlstra <peterz@infradead.org> - 2016-12-01 17:30 +0100
  [PATCH v2 11/11] [rfc] locking/ww_mutex: Always spin optimistically for the first waiter Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100
  [PATCH v2 06/11] locking/ww_mutex: Notify waiters that have to back off while adding tasks to wait list Nicolai Hähnle <nhaehnle@gmail.com> - 2016-12-01 15:10 +0100

csiph-web