Re: Deadlock in net/sunrpc/sched.c

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Thu, 2006-03-02 at 12:45 +0100, Simon Derr wrote:
> On Thu, 2 Mar 2006, Frederik Deweerdt wrote:
> 
> > On Thu, Mar 02, 2006 at 11:38:10AM +0100, Simon Derr wrote:
> > > This happened with 2.6.12 but it seems that the code has not changed and 
> > > the issue is very probably still present in the current kernels.
> > Looks like it's fixed in 2.6.16-rc5, could you check agains the current
> > tree?
> 
> No, the code is still the same.
> 
> > Hmm, not sure this will even compile...
> 
> oops....
> 
> Index: linux-2.6.12.6/net/sunrpc/sched.c
> ===================================================================
> --- linux-2.6.12.6.orig/net/sunrpc/sched.c	2005-08-29 18:55:27.000000000 +0200
> +++ linux-2.6.12.6/net/sunrpc/sched.c	2006-03-02 12:41:42.000000000 +0100
> @@ -400,16 +400,16 @@ __rpc_default_timer(struct rpc_task *tas
>   */
>  void rpc_wake_up_task(struct rpc_task *task)
>  {
> +	struct rpc_wait_queue *queue = task->u.tk_wait.rpc_waitq;
> +	spin_lock_bh(&queue->lock);
>  	if (rpc_start_wakeup(task)) {
>  		if (RPC_IS_QUEUED(task)) {
> -			struct rpc_wait_queue *queue = task->u.tk_wait.rpc_waitq;
>  
> -			spin_lock_bh(&queue->lock);
>  			__rpc_do_wake_up_task(task);
> -			spin_unlock_bh(&queue->lock);
>  		}
>  		rpc_finish_wakeup(task);
>  	}
> +	spin_unlock_bh(&queue->lock);
>  }


Bzzzt... Race: How do you know that task->u.tk_wait.rpc_waitq contains a
valid pointer to a queue before you call rpc_start_wakeup()?

Cheers,
  Trond

>  /*
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/