On Mon, 19 Mar 2012 14:16:42 -0400
Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> The problem is that for the case of priority queues, we
> have to assume that __rpc_remove_wait_queue_priority will move new
> elements from the tk_wait.links lists into the queue->tasks[] list.
> We therefore cannot use list_for_each_entry_safe() on queue->tasks[],
> since that will skip these new tasks that __rpc_remove_wait_queue_priority
> is adding.
>
> Without this fix, rpc_wake_up and rpc_wake_up_status will both fail
> to wake up all functions on priority wait queues, which can result
> in some nasty hangs.
>
> Reported-by: Andy Adamson <andros@xxxxxxxxxx>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> net/sunrpc/sched.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index 1c570a8..994cfea 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -534,14 +534,18 @@ EXPORT_SYMBOL_GPL(rpc_wake_up_next);
> */
> void rpc_wake_up(struct rpc_wait_queue *queue)
> {
> - struct rpc_task *task, *next;
> struct list_head *head;
>
> spin_lock_bh(&queue->lock);
> head = &queue->tasks[queue->maxpriority];
> for (;;) {
> - list_for_each_entry_safe(task, next, head, u.tk_wait.list)
> + while (!list_empty(head)) {
> + struct rpc_task *task;
> + task = list_first_entry(head,
> + struct rpc_task,
> + u.tk_wait.list);
> rpc_wake_up_task_queue_locked(queue, task);
> + }
> if (head == &queue->tasks[0])
> break;
> head--;
> @@ -559,13 +563,16 @@ EXPORT_SYMBOL_GPL(rpc_wake_up);
> */
> void rpc_wake_up_status(struct rpc_wait_queue *queue, int status)
> {
> - struct rpc_task *task, *next;
> struct list_head *head;
>
> spin_lock_bh(&queue->lock);
> head = &queue->tasks[queue->maxpriority];
> for (;;) {
> - list_for_each_entry_safe(task, next, head, u.tk_wait.list) {
> + while (!list_empty(head)) {
> + struct rpc_task *task;
> + task = list_first_entry(head,
> + struct rpc_task,
> + u.tk_wait.list);
> task->tk_status = status;
> rpc_wake_up_task_queue_locked(queue, task);
> }
I have a question about this (old) patch...
Steve D. backported this patch for RHEL5, and it's causing a deadlock
there. This is mainly due to the fact that this code in RHEL5 is based
on 2.6.18 and is quite different (it still has the RPC_TASK_WAKEUP
flag, and there's an inversion problem with it in the code). I don't
think it's a problem in most later kernels.
What I'm not clear on is this: how can new entries end up on these lists
even though we hold the queue->lock here? It looks like the queue->lock
is always held when __rpc_remove_wait_queue_priority is called.
Even if there is some way for that to happen, how is it safe to add
entries to that list without taking the lock? That seems like it would
be a great way to corrupt the list.
Thanks,
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
