lkml.org 
[lkml]   [2010]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2010-12-22 07:53    [W:0.684 / U:0.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site