Messages in this thread | | | Date | Tue, 21 Dec 2010 22:50:14 -0800 | From | Darren Hart <> | Subject | Re: [PATCH 1/4 V3 ] futex,plist: pass the real head of the priority list to plist_del() |
| |
On 12/21/2010 10:18 PM, Lai Jiangshan wrote: > On 12/22/2010 03:13 AM, Darren Hart wrote: >> On 12/21/2010 01:55 AM, Lai Jiangshan wrote: >> >> Hi Lai, >> >> Looks about ready to me, only a couple of more nitpics from me. >> > > Fixed, thanks.
Looks good to me. How did the futextest runs go?
-- Darren
> > Subject: [PATCH 1/4 V3 ] futex,plist: pass the real head of the priority list to plist_del() > > Some plist_del()s in kernel/futex.c are passed a faked head of the > priority list. > > It does not fail because the current code does not require the real head > in plist_del(). The current code of plist_del() just uses the head for checking, > so it will not cause a bad result even when we use a faked head. > > But it is undocumented usage: > > /** > * plist_del - Remove a @node from plist. > * > * @node: &struct plist_node pointer - entry to be removed > * @head: &struct plist_head pointer - list head > */ > > The document said that @head is "list head" the head of the priority list. > > In futex code, several places use "plist_del(&q->list,&q->list.plist);", > they passes faked head, we fix them all. > > Thanks to Darren Hart for many suggestions. > > Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com> > --- > diff --git a/kernel/futex.c b/kernel/futex.c > index 3019b92..d4f7252 100644 > --- a/kernel/futex.c > +++ b/kernel/futex.c > @@ -740,6 +740,24 @@ retry: > return ret; > } > > +/** > + * __unqueue_futex() - Remove the futex_q from its futex_hash_bucket > + * @q: The futex_q to unqueue > + * > + * The q->lock_ptr must not be NULL and must be held by the caller. > + */ > +static void __unqueue_futex(struct futex_q *q) > +{ > + struct futex_hash_bucket *hb; > + > + if (WARN_ON(!q->lock_ptr || !spin_is_locked(q->lock_ptr) > + || plist_node_empty(&q->list))) > + return; > + > + hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock); > + plist_del(&q->list,&hb->chain); > +} > + > /* > * The hash bucket lock must be held when this is called. > * Afterwards, the futex_q must not be accessed. > @@ -757,7 +775,7 @@ static void wake_futex(struct futex_q *q) > */ > get_task_struct(p); > > - plist_del(&q->list,&q->list.plist); > + __unqueue_futex(q); > /* > * The waiting task can free the futex_q as soon as > * q->lock_ptr = NULL is written, without taking any locks. A > @@ -1066,8 +1084,7 @@ void requeue_pi_wake_futex(struct futex_q *q, union futex_key *key, > get_futex_key_refs(key); > q->key = *key; > > - WARN_ON(plist_node_empty(&q->list)); > - plist_del(&q->list,&q->list.plist); > + __unqueue_futex(q); > > WARN_ON(!q->rt_waiter); > q->rt_waiter = NULL; > @@ -1470,8 +1487,7 @@ retry: > spin_unlock(lock_ptr); > goto retry; > } > - WARN_ON(plist_node_empty(&q->list)); > - plist_del(&q->list,&q->list.plist); > + __unqueue_futex(q); > > BUG_ON(q->pi_state); > > @@ -1491,8 +1507,7 @@ retry: > static void unqueue_me_pi(struct futex_q *q) > __releases(q->lock_ptr) > { > - WARN_ON(plist_node_empty(&q->list)); > - plist_del(&q->list,&q->list.plist); > + __unqueue_futex(q); > > BUG_ON(!q->pi_state); > free_pi_state(q->pi_state); > @@ -2133,7 +2148,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb, > * We were woken prior to requeue by a timeout or a signal. > * Unqueue the futex_q and determine which it was. > */ > - plist_del(&q->list,&q->list.plist); > + plist_del(&q->list,&hb->chain); > > /* Handle spurious wakeups gracefully */ > ret = -EWOULDBLOCK;
-- Darren Hart Yocto Linux Kernel
| |