Skip to content

Commit

Permalink
rtmutex: Make wait_lock irq safe
Browse files Browse the repository at this point in the history
Sasha reported a lockdep splat about a potential deadlock between RCU boosting
rtmutex and the posix timer it_lock.

CPU0					CPU1

rtmutex_lock(&rcu->rt_mutex)
  spin_lock(&rcu->rt_mutex.wait_lock)
					local_irq_disable()
					spin_lock(&timer->it_lock)
					spin_lock(&rcu->mutex.wait_lock)
--> Interrupt
    spin_lock(&timer->it_lock)

This is caused by the following code sequence on CPU1

     rcu_read_lock()
     x = lookup();
     if (x)
     	spin_lock_irqsave(&x->it_lock);
     rcu_read_unlock();
     return x;

We could fix that in the posix timer code by keeping rcu read locked across
the spinlocked and irq disabled section, but the above sequence is common and
there is no reason not to support it.

Taking rt_mutex.wait_lock irq safe prevents the deadlock.

Reported-by: Sasha Levin <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul McKenney <[email protected]>
  • Loading branch information
KAGA-KOKO committed Jan 26, 2016
1 parent 92e963f commit b4abf91
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 72 deletions.
18 changes: 9 additions & 9 deletions kernel/futex.c
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this,
if (pi_state->owner != current)
return -EINVAL;

raw_spin_lock(&pi_state->pi_mutex.wait_lock);
raw_spin_lock_irq(&pi_state->pi_mutex.wait_lock);
new_owner = rt_mutex_next_owner(&pi_state->pi_mutex);

/*
Expand All @@ -1217,22 +1217,22 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval, struct futex_q *this,
else if (curval != uval)
ret = -EINVAL;
if (ret) {
raw_spin_unlock(&pi_state->pi_mutex.wait_lock);
raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock);
return ret;
}

raw_spin_lock_irq(&pi_state->owner->pi_lock);
raw_spin_lock(&pi_state->owner->pi_lock);
WARN_ON(list_empty(&pi_state->list));
list_del_init(&pi_state->list);
raw_spin_unlock_irq(&pi_state->owner->pi_lock);
raw_spin_unlock(&pi_state->owner->pi_lock);

raw_spin_lock_irq(&new_owner->pi_lock);
raw_spin_lock(&new_owner->pi_lock);
WARN_ON(!list_empty(&pi_state->list));
list_add(&pi_state->list, &new_owner->pi_state_list);
pi_state->owner = new_owner;
raw_spin_unlock_irq(&new_owner->pi_lock);
raw_spin_unlock(&new_owner->pi_lock);

raw_spin_unlock(&pi_state->pi_mutex.wait_lock);
raw_spin_unlock_irq(&pi_state->pi_mutex.wait_lock);

deboost = rt_mutex_futex_unlock(&pi_state->pi_mutex, &wake_q);

Expand Down Expand Up @@ -2127,11 +2127,11 @@ static int fixup_owner(u32 __user *uaddr, struct futex_q *q, int locked)
* we returned due to timeout or signal without taking the
* rt_mutex. Too late.
*/
raw_spin_lock(&q->pi_state->pi_mutex.wait_lock);
raw_spin_lock_irq(&q->pi_state->pi_mutex.wait_lock);
owner = rt_mutex_owner(&q->pi_state->pi_mutex);
if (!owner)
owner = rt_mutex_next_owner(&q->pi_state->pi_mutex);
raw_spin_unlock(&q->pi_state->pi_mutex.wait_lock);
raw_spin_unlock_irq(&q->pi_state->pi_mutex.wait_lock);
ret = fixup_pi_state_owner(uaddr, q, owner);
goto out;
}
Expand Down
Loading

0 comments on commit b4abf91

Please sign in to comment.