Re: [patch 2/5] futex: Use futex_top_waiter() in lookup_pi_state()
On Wed, 2014-06-11 at 20:45 +, Thomas Gleixner wrote: > No point in open coding the same function again. > > Signed-off-by: Thomas Gleixner > --- > kernel/futex.c | 128 > - > 1 file changed, 63 insertions(+), 65 deletions(-) > > Index: linux/kernel/futex.c > === > --- linux.orig/kernel/futex.c > +++ linux/kernel/futex.c > @@ -796,87 +796,85 @@ static int > lookup_pi_state(u32 uval, struct futex_hash_bucket *hb, > union futex_key *key, struct futex_pi_state **ps) > { > + struct futex_q *match = futex_top_waiter(hb, key); > struct futex_pi_state *pi_state = NULL; > - struct futex_q *this, *next; > struct task_struct *p; > pid_t pid = uval & FUTEX_TID_MASK; > > - plist_for_each_entry_safe(this, next, >chain, list) { > - if (match_futex(>key, key)) { > + if (match) { > + /* > +* Sanity check the waiter before increasing the > +* refcount and attaching to it. > +*/ > + pi_state = match->pi_state; > + /* > +* Userspace might have messed up non-PI and PI > +* futexes [3] > +*/ > + if (unlikely(!pi_state)) > + return -EINVAL; > + > + WARN_ON(!atomic_read(_state->refcount)); > + > + /* > +* Handle the owner died case: > +*/ > + if (uval & FUTEX_OWNER_DIED) { > /* > -* Sanity check the waiter before increasing > -* the refcount and attaching to it. > +* exit_pi_state_list sets owner to NULL and > +* wakes the topmost waiter. The task which > +* acquires the pi_state->rt_mutex will fixup > +* owner. > */ > - pi_state = this->pi_state; > - /* > -* Userspace might have messed up non-PI and > -* PI futexes [3] > -*/ > - if (unlikely(!pi_state)) > - return -EINVAL; > - > - WARN_ON(!atomic_read(_state->refcount)); > - > - /* > -* Handle the owner died case: > -*/ > - if (uval & FUTEX_OWNER_DIED) { > - /* > -* exit_pi_state_list sets owner to > NULL and > -* wakes the topmost waiter. The task > which > -* acquires the pi_state->rt_mutex > will fixup > -* owner. > -*/ > - if (!pi_state->owner) { > - /* > -* No pi state owner, but the > user > -* space TID is not 0. > Inconsistent > -* state. [5] > -*/ > - if (pid) > - return -EINVAL; > - /* > -* Take a ref on the state and > -* return. [4] > -*/ > - goto out_state; > - } > - > + if (!pi_state->owner) { > /* > -* If TID is 0, then either the dying > owner > -* has not yet executed > exit_pi_state_list() > -* or some waiter acquired the rtmutex > in the > -* pi state, but did not yet fixup the > TID in > -* user space. > -* > -* Take a ref on the state and return. > [6] > +* No pi state owner, but the user > +* space TID is not 0. Inconsistent > +* state. [5] > */ > - if (!pid) > - goto out_state; > - } else { > + if (pid) > + return -EINVAL; > /* > -* If the owner died bit is not set, > -* then
Re: [patch 2/5] futex: Use futex_top_waiter() in lookup_pi_state()
On Wed, 2014-06-11 at 20:45 +, Thomas Gleixner wrote: No point in open coding the same function again. Signed-off-by: Thomas Gleixner t...@linutronix.de --- kernel/futex.c | 128 - 1 file changed, 63 insertions(+), 65 deletions(-) Index: linux/kernel/futex.c === --- linux.orig/kernel/futex.c +++ linux/kernel/futex.c @@ -796,87 +796,85 @@ static int lookup_pi_state(u32 uval, struct futex_hash_bucket *hb, union futex_key *key, struct futex_pi_state **ps) { + struct futex_q *match = futex_top_waiter(hb, key); struct futex_pi_state *pi_state = NULL; - struct futex_q *this, *next; struct task_struct *p; pid_t pid = uval FUTEX_TID_MASK; - plist_for_each_entry_safe(this, next, hb-chain, list) { - if (match_futex(this-key, key)) { + if (match) { + /* +* Sanity check the waiter before increasing the +* refcount and attaching to it. +*/ + pi_state = match-pi_state; + /* +* Userspace might have messed up non-PI and PI +* futexes [3] +*/ + if (unlikely(!pi_state)) + return -EINVAL; + + WARN_ON(!atomic_read(pi_state-refcount)); + + /* +* Handle the owner died case: +*/ + if (uval FUTEX_OWNER_DIED) { /* -* Sanity check the waiter before increasing -* the refcount and attaching to it. +* exit_pi_state_list sets owner to NULL and +* wakes the topmost waiter. The task which +* acquires the pi_state-rt_mutex will fixup +* owner. */ - pi_state = this-pi_state; - /* -* Userspace might have messed up non-PI and -* PI futexes [3] -*/ - if (unlikely(!pi_state)) - return -EINVAL; - - WARN_ON(!atomic_read(pi_state-refcount)); - - /* -* Handle the owner died case: -*/ - if (uval FUTEX_OWNER_DIED) { - /* -* exit_pi_state_list sets owner to NULL and -* wakes the topmost waiter. The task which -* acquires the pi_state-rt_mutex will fixup -* owner. -*/ - if (!pi_state-owner) { - /* -* No pi state owner, but the user -* space TID is not 0. Inconsistent -* state. [5] -*/ - if (pid) - return -EINVAL; - /* -* Take a ref on the state and -* return. [4] -*/ - goto out_state; - } - + if (!pi_state-owner) { /* -* If TID is 0, then either the dying owner -* has not yet executed exit_pi_state_list() -* or some waiter acquired the rtmutex in the -* pi state, but did not yet fixup the TID in -* user space. -* -* Take a ref on the state and return. [6] +* No pi state owner, but the user +* space TID is not 0. Inconsistent +* state. [5] */ - if (!pid) - goto out_state; - } else { + if (pid) + return -EINVAL; /* -* If the owner died bit is not set, -* then the pi_state must have an -* owner. [7] +
[patch 2/5] futex: Use futex_top_waiter() in lookup_pi_state()
No point in open coding the same function again. Signed-off-by: Thomas Gleixner --- kernel/futex.c | 128 - 1 file changed, 63 insertions(+), 65 deletions(-) Index: linux/kernel/futex.c === --- linux.orig/kernel/futex.c +++ linux/kernel/futex.c @@ -796,87 +796,85 @@ static int lookup_pi_state(u32 uval, struct futex_hash_bucket *hb, union futex_key *key, struct futex_pi_state **ps) { + struct futex_q *match = futex_top_waiter(hb, key); struct futex_pi_state *pi_state = NULL; - struct futex_q *this, *next; struct task_struct *p; pid_t pid = uval & FUTEX_TID_MASK; - plist_for_each_entry_safe(this, next, >chain, list) { - if (match_futex(>key, key)) { + if (match) { + /* +* Sanity check the waiter before increasing the +* refcount and attaching to it. +*/ + pi_state = match->pi_state; + /* +* Userspace might have messed up non-PI and PI +* futexes [3] +*/ + if (unlikely(!pi_state)) + return -EINVAL; + + WARN_ON(!atomic_read(_state->refcount)); + + /* +* Handle the owner died case: +*/ + if (uval & FUTEX_OWNER_DIED) { /* -* Sanity check the waiter before increasing -* the refcount and attaching to it. +* exit_pi_state_list sets owner to NULL and +* wakes the topmost waiter. The task which +* acquires the pi_state->rt_mutex will fixup +* owner. */ - pi_state = this->pi_state; - /* -* Userspace might have messed up non-PI and -* PI futexes [3] -*/ - if (unlikely(!pi_state)) - return -EINVAL; - - WARN_ON(!atomic_read(_state->refcount)); - - /* -* Handle the owner died case: -*/ - if (uval & FUTEX_OWNER_DIED) { - /* -* exit_pi_state_list sets owner to NULL and -* wakes the topmost waiter. The task which -* acquires the pi_state->rt_mutex will fixup -* owner. -*/ - if (!pi_state->owner) { - /* -* No pi state owner, but the user -* space TID is not 0. Inconsistent -* state. [5] -*/ - if (pid) - return -EINVAL; - /* -* Take a ref on the state and -* return. [4] -*/ - goto out_state; - } - + if (!pi_state->owner) { /* -* If TID is 0, then either the dying owner -* has not yet executed exit_pi_state_list() -* or some waiter acquired the rtmutex in the -* pi state, but did not yet fixup the TID in -* user space. -* -* Take a ref on the state and return. [6] +* No pi state owner, but the user +* space TID is not 0. Inconsistent +* state. [5] */ - if (!pid) - goto out_state; - } else { + if (pid) + return -EINVAL; /* -* If the owner died bit is not set, -* then the pi_state must have an -* owner. [7] +* Take a ref on the state and +* return. [4] */ - if (!pi_state->owner) -
[patch 2/5] futex: Use futex_top_waiter() in lookup_pi_state()
No point in open coding the same function again. Signed-off-by: Thomas Gleixner t...@linutronix.de --- kernel/futex.c | 128 - 1 file changed, 63 insertions(+), 65 deletions(-) Index: linux/kernel/futex.c === --- linux.orig/kernel/futex.c +++ linux/kernel/futex.c @@ -796,87 +796,85 @@ static int lookup_pi_state(u32 uval, struct futex_hash_bucket *hb, union futex_key *key, struct futex_pi_state **ps) { + struct futex_q *match = futex_top_waiter(hb, key); struct futex_pi_state *pi_state = NULL; - struct futex_q *this, *next; struct task_struct *p; pid_t pid = uval FUTEX_TID_MASK; - plist_for_each_entry_safe(this, next, hb-chain, list) { - if (match_futex(this-key, key)) { + if (match) { + /* +* Sanity check the waiter before increasing the +* refcount and attaching to it. +*/ + pi_state = match-pi_state; + /* +* Userspace might have messed up non-PI and PI +* futexes [3] +*/ + if (unlikely(!pi_state)) + return -EINVAL; + + WARN_ON(!atomic_read(pi_state-refcount)); + + /* +* Handle the owner died case: +*/ + if (uval FUTEX_OWNER_DIED) { /* -* Sanity check the waiter before increasing -* the refcount and attaching to it. +* exit_pi_state_list sets owner to NULL and +* wakes the topmost waiter. The task which +* acquires the pi_state-rt_mutex will fixup +* owner. */ - pi_state = this-pi_state; - /* -* Userspace might have messed up non-PI and -* PI futexes [3] -*/ - if (unlikely(!pi_state)) - return -EINVAL; - - WARN_ON(!atomic_read(pi_state-refcount)); - - /* -* Handle the owner died case: -*/ - if (uval FUTEX_OWNER_DIED) { - /* -* exit_pi_state_list sets owner to NULL and -* wakes the topmost waiter. The task which -* acquires the pi_state-rt_mutex will fixup -* owner. -*/ - if (!pi_state-owner) { - /* -* No pi state owner, but the user -* space TID is not 0. Inconsistent -* state. [5] -*/ - if (pid) - return -EINVAL; - /* -* Take a ref on the state and -* return. [4] -*/ - goto out_state; - } - + if (!pi_state-owner) { /* -* If TID is 0, then either the dying owner -* has not yet executed exit_pi_state_list() -* or some waiter acquired the rtmutex in the -* pi state, but did not yet fixup the TID in -* user space. -* -* Take a ref on the state and return. [6] +* No pi state owner, but the user +* space TID is not 0. Inconsistent +* state. [5] */ - if (!pid) - goto out_state; - } else { + if (pid) + return -EINVAL; /* -* If the owner died bit is not set, -* then the pi_state must have an -* owner. [7] +* Take a ref on the state and +* return. [4] */ - if (!pi_state-owner) -