Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1500345
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop |
| Date | 2016-10-13 17:20 +0200 |
| Message-ID | <srPXB-5cV-37@gated-at.bofh.it> (permalink) |
| References | <spEWB-2dt-5@gated-at.bofh.it> <spFfX-2lv-15@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Peter,
I'm struggling to get my head around the handoff code after this change...
On Fri, Oct 07, 2016 at 04:52:49PM +0200, Peter Zijlstra wrote:
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -631,13 +631,21 @@ __mutex_lock_common(struct mutex *lock,
>
> lock_contended(&lock->dep_map, ip);
>
> + set_task_state(task, state);
> for (;;) {
> + /*
> + * Once we hold wait_lock, we're serialized against
> + * mutex_unlock() handing the lock off to us, do a trylock
> + * before testing the error conditions to make sure we pick up
> + * the handoff.
> + */
> if (__mutex_trylock(lock, first))
> - break;
> + goto acquired;
>
> /*
> - * got a signal? (This code gets eliminated in the
> - * TASK_UNINTERRUPTIBLE case.)
> + * Check for signals and wound conditions while holding
> + * wait_lock. This ensures the lock cancellation is ordered
> + * against mutex_unlock() and wake-ups do not go missing.
> */
> if (unlikely(signal_pending_state(state, task))) {
> ret = -EINTR;
> @@ -650,16 +658,27 @@ __mutex_lock_common(struct mutex *lock,
> goto err;
> }
>
> - __set_task_state(task, state);
> spin_unlock_mutex(&lock->wait_lock, flags);
> schedule_preempt_disabled();
> - spin_lock_mutex(&lock->wait_lock, flags);
>
> if (!first && __mutex_waiter_is_first(lock, &waiter)) {
> first = true;
> __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF);
> }
> +
> + set_task_state(task, state);
With this change, we no longer hold the lock wit_hen we set the task
state, and it's ordered strictly *after* setting the HANDOFF flag.
Doesn't that mean that the unlock code can see the HANDOFF flag, issue
the wakeup, but then we come in and overwrite the task state?
I'm struggling to work out whether that's an issue, but it certainly
feels odd and is a change from the previous behaviour.
Will
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH -v4 0/8] locking/mutex: Rewrite basic mutex Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:10 +0200
[PATCH -v4 4/8] locking/mutex: Allow MUTEX_SPIN_ON_OWNER when DEBUG_MUTEXES Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:10 +0200
[PATCH -v4 7/8] locking/mutex: Simplify some ww_mutex code in __mutex_lock_common() Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:10 +0200
[PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:30 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Will Deacon <will.deacon@arm.com> - 2016-10-13 17:20 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-17 12:50 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-17 15:30 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Boqun Feng <boqun.feng@gmail.com> - 2016-10-17 15:50 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-17 18:00 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-19 19:40 +0200
ciao set_task_state() (was Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop) Davidlohr Bueso <dave@stgolabs.net> - 2016-10-24 04:00 +0200
Re: ciao set_task_state() (was Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop) Kent Overstreet <kent.overstreet@gmail.com> - 2016-10-24 15:30 +0200
Re: ciao set_task_state() (was Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop) Kent Overstreet <kent.overstreet@gmail.com> - 2016-10-24 16:30 +0200
Re: ciao set_task_state() (was Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop) Eric Wheeler <bcache@lists.ewheeler.net> - 2016-10-25 19:00 +0200
Re: ciao set_task_state() (was Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop) Kent Overstreet <kent.overstreet@gmail.com> - 2016-10-25 19:50 +0200
Re: [PATCH -v4 6/8] locking/mutex: Restructure wait loop Peter Zijlstra <peterz@infradead.org> - 2016-10-18 15:20 +0200
[PATCH -v4 8/8] locking/mutex: Enable optimistic spinning of woken waiter Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:30 +0200
Re: [PATCH -v4 8/8] locking/mutex: Enable optimistic spinning of woken waiter Will Deacon <will.deacon@arm.com> - 2016-10-13 17:30 +0200
Re: [PATCH -v4 8/8] locking/mutex: Enable optimistic spinning of woken waiter Peter Zijlstra <peterz@infradead.org> - 2016-10-17 11:40 +0200
Re: [PATCH -v4 8/8] locking/mutex: Enable optimistic spinning of woken waiter Peter Zijlstra <peterz@infradead.org> - 2016-10-18 14:30 +0200
[PATCH -v4 2/8] locking/mutex: Rework mutex::owner Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:30 +0200
Re: [PATCH -v4 2/8] locking/mutex: Rework mutex::owner Davidlohr Bueso <dave@stgolabs.net> - 2016-10-12 20:40 +0200
Re: [PATCH -v4 2/8] locking/mutex: Rework mutex::owner Jason Low <jason.low2@hpe.com> - 2016-10-12 22:00 +0200
Re: [PATCH -v4 2/8] locking/mutex: Rework mutex::owner Will Deacon <will.deacon@arm.com> - 2016-10-13 17:20 +0200
[PATCH -v4 5/8] locking/mutex: Add lock handoff to avoid starvation Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:40 +0200
Re: [PATCH -v4 5/8] locking/mutex: Add lock handoff to avoid starvation Will Deacon <will.deacon@arm.com> - 2016-10-13 17:20 +0200
Re: [PATCH -v4 5/8] locking/mutex: Add lock handoff to avoid starvation Peter Zijlstra <peterz@infradead.org> - 2016-10-17 11:30 +0200
Re: [PATCH -v4 5/8] locking/mutex: Add lock handoff to avoid starvation Peter Zijlstra <peterz@infradead.org> - 2016-10-18 14:40 +0200
Re: [PATCH -v4 5/8] locking/mutex: Add lock handoff to avoid starvation Peter Zijlstra <peterz@infradead.org> - 2016-10-18 15:10 +0200
[PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:40 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:50 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-07 18:00 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-07 18:20 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Thomas Gleixner <tglx@linutronix.de> - 2016-10-08 14:10 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Thomas Gleixner <tglx@linutronix.de> - 2016-10-08 16:20 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-08 18:50 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-08 16:20 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-18 14:50 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Peter Zijlstra <peterz@infradead.org> - 2016-10-18 15:00 +0200
Re: [PATCH -v4 1/8] locking/drm: Kill mutex trickery Chris Wilson <chris@chris-wilson.co.uk> - 2016-10-18 15:00 +0200
[PATCH -v4 3/8] locking/mutex: Kill arch specific code Peter Zijlstra <peterz@infradead.org> - 2016-10-07 17:40 +0200
Re: [PATCH -v4 0/8] locking/mutex: Rewrite basic mutex Linus Torvalds <torvalds@linux-foundation.org> - 2016-10-07 17:50 +0200
Re: [PATCH -v4 0/8] locking/mutex: Rewrite basic mutex Jason Low <jason.low2@hpe.com> - 2016-10-11 21:10 +0200
csiph-web