Messages in this thread Patch in this message | | | From | Julia Cartwright <> | Subject | [PATCH] futex: Drop now unnecessary check in exit_pi_state() | Date | Fri, 3 Nov 2017 17:30:28 -0500 |
| |
This check was an attempt to protect against a race with put_pi_state() by ensuring that the pi_state_list was consistent across the unlock/lock of pi_lock.
However, as of commit 153fbd1226fb3 ("futex: Fix more put_pi_state() vs. exit_pi_state_list() races"), this check is no longer necessary because we now hold a reference to the pi_state object across the unlock/lock of pi_lock. This reference guarantees that a put_pi_state() on another CPU won't rip the pi_state object from the list when we drop pi_lock.
Cc: Gratian Crisan <gratian.crisan@ni.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Darren Hart <dvhart@infradead.org> Signed-off-by: Julia Cartwright <julia@ni.com> --- I'm not sure my analysis is 100% correct here, so please carefully think through it, as I'm sure you all always do when futex patches hit your mailbox :).
Julia
kernel/futex.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/kernel/futex.c b/kernel/futex.c index ca5bb9cba5cf..e127ec0555b6 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -889,7 +889,7 @@ static struct task_struct *futex_find_get_task(pid_t pid) */ void exit_pi_state_list(struct task_struct *curr) { - struct list_head *next, *head = &curr->pi_state_list; + struct list_head *head = &curr->pi_state_list; struct futex_pi_state *pi_state; struct futex_hash_bucket *hb; union futex_key key = FUTEX_KEY_INIT; @@ -903,8 +903,7 @@ void exit_pi_state_list(struct task_struct *curr) */ raw_spin_lock_irq(&curr->pi_lock); while (!list_empty(head)) { - next = head->next; - pi_state = list_entry(next, struct futex_pi_state, list); + pi_state = list_first_entry(head, struct futex_pi_state, list); key = pi_state->key; hb = hash_futex(&key); @@ -929,17 +928,6 @@ void exit_pi_state_list(struct task_struct *curr) spin_lock(&hb->lock); raw_spin_lock_irq(&pi_state->pi_mutex.wait_lock); raw_spin_lock(&curr->pi_lock); - /* - * We dropped the pi-lock, so re-check whether this - * task still owns the PI-state: - */ - if (head->next != next) { - /* retain curr->pi_lock for the loop invariant */ - raw_spin_unlock(&pi_state->pi_mutex.wait_lock); - spin_unlock(&hb->lock); - put_pi_state(pi_state); - continue; - } WARN_ON(pi_state->owner != curr); WARN_ON(list_empty(&pi_state->list)); -- 2.14.2
| |