lkml.org 
[lkml]   [2016]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -v4 6/8] locking/mutex: Restructure wait loop
    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

    \
     
     \ /
      Last update: 2016-10-13 17:18    [W:4.149 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site