On Wed, 16 Feb 2011 09:26:13 -0500
Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:
> On Wed, 2011-02-16 at 09:09 -0500, Trond Myklebust wrote:
> > On Tue, 2011-02-15 at 18:47 -0500, Trond Myklebust wrote:
> > > diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> > > index 243fc09..11b71b1 100644
> > > --- a/net/sunrpc/sched.c
> > > +++ b/net/sunrpc/sched.c
> > > @@ -252,23 +252,39 @@ static void rpc_set_active(struct rpc_task *task)
> > >
> > > /*
> > > * Mark an RPC call as having completed by clearing the 'active' bit
> > > + * and then waking up all tasks that were sleeping.
> > > */
> > > -static void rpc_mark_complete_task(struct rpc_task *task)
> > > +static int rpc_complete_task(struct rpc_task *task)
> > > {
> > > - smp_mb__before_clear_bit();
> > > + void *m = &task->tk_runstate;
> > > + wait_queue_head_t *wq = bit_waitqueue(m, RPC_TASK_ACTIVE);
> > > + struct wait_bit_key k = __WAIT_BIT_KEY_INITIALIZER(m, RPC_TASK_ACTIVE);
> > > + unsigned long flags;
> > > + int ret;
> > > +
> > > + spin_lock_irqsave(&wq->lock, flags);
> > > clear_bit(RPC_TASK_ACTIVE, &task->tk_runstate);
> > > - smp_mb__after_clear_bit();
> > > - wake_up_bit(&task->tk_runstate, RPC_TASK_ACTIVE);
> > > + if (waitqueue_active(wq))
> > > + __wake_up_locked_key(wq, TASK_NORMAL, &k);
> > > + ret = atomic_dec_and_test(&task->tk_count);
> > > + spin_unlock_irqrestore(&wq->lock, flags);
> > > + return ret;
> > > }
> > >
> > > /*
> > > * Allow callers to wait for completion of an RPC call
> > > + *
> > > + * Note the use of out_of_line_wait_on_bit() rather than wait_on_bit()
> > > + * to enforce taking of the wq->lock and hence avoid races with
> > > + * rpc_complete_task().
> > > */
> > > int __rpc_wait_for_completion_task(struct rpc_task *task, int (*action)(void *))
> > > {
> > > + BUG_ON(!RPC_IS_ASYNC(task));
> > > +
> > > if (action == NULL)
> > > action = rpc_wait_bit_killable;
> > > - return wait_on_bit(&task->tk_runstate, RPC_TASK_ACTIVE,
> > > + return out_of_line_wait_on_bit(&task->tk_runstate, RPC_TASK_ACTIVE,
> > > action, TASK_KILLABLE);
> > > }
> > > EXPORT_SYMBOL_GPL(__rpc_wait_for_completion_task);
> >
> > Never mind. The above ordering scheme is broken by the fact that
> > 'wake_bit_function' calls autoremove_wake_function, which again means
> > that finish_wait optimises away the spin lock.
> >
> > Back to the drawing board...
>
> ...and after yet another cup of coffee the solution suggests itself: we
> just need to reorder the calls to __wake_up_locked_key and the
> atomic_dec_and_test in rpc_complete_task. The revised patch is appended.
>
> Cheers
> Trond
>
Thanks Trond,
This builds, but I can't plug in the module:
[ 103.540405] sunrpc: Unknown symbol __wake_up_locked_key (err 0)
...I think __wake_up_locked_key will need to be exported too. I'll do
that and then test this out later today.
--
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
