Commit eb9b55ab authored by Trond Myklebust's avatar Trond Myklebust

SUNRPC: Tighten up the task locking rules in __rpc_execute()

We should probably not be testing any flags after we've cleared the
RPC_TASK_RUNNING flag, since rpc_make_runnable() is then free to assign the
rpc_task to another workqueue, which may then destroy it.

We can fix any races with rpc_make_runnable() by ensuring that we only
clear the RPC_TASK_RUNNING flag while holding the rpc_wait_queue->lock that
the task is supposed to be sleeping on (and then checking whether or not
the task really is sleeping).
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 16b71fdf
...@@ -293,11 +293,6 @@ static void rpc_make_runnable(struct rpc_task *task) ...@@ -293,11 +293,6 @@ static void rpc_make_runnable(struct rpc_task *task)
rpc_clear_queued(task); rpc_clear_queued(task);
if (rpc_test_and_set_running(task)) if (rpc_test_and_set_running(task))
return; return;
/* We might have raced */
if (RPC_IS_QUEUED(task)) {
rpc_clear_running(task);
return;
}
if (RPC_IS_ASYNC(task)) { if (RPC_IS_ASYNC(task)) {
int status; int status;
...@@ -607,7 +602,9 @@ void rpc_release_calldata(const struct rpc_call_ops *ops, void *calldata) ...@@ -607,7 +602,9 @@ void rpc_release_calldata(const struct rpc_call_ops *ops, void *calldata)
*/ */
static void __rpc_execute(struct rpc_task *task) static void __rpc_execute(struct rpc_task *task)
{ {
int status = 0; struct rpc_wait_queue *queue;
int task_is_async = RPC_IS_ASYNC(task);
int status = 0;
dprintk("RPC: %5u __rpc_execute flags=0x%x\n", dprintk("RPC: %5u __rpc_execute flags=0x%x\n",
task->tk_pid, task->tk_flags); task->tk_pid, task->tk_flags);
...@@ -647,15 +644,25 @@ static void __rpc_execute(struct rpc_task *task) ...@@ -647,15 +644,25 @@ static void __rpc_execute(struct rpc_task *task)
*/ */
if (!RPC_IS_QUEUED(task)) if (!RPC_IS_QUEUED(task))
continue; continue;
rpc_clear_running(task); /*
if (RPC_IS_ASYNC(task)) { * The queue->lock protects against races with
/* Careful! we may have raced... */ * rpc_make_runnable().
if (RPC_IS_QUEUED(task)) *
return; * Note that once we clear RPC_TASK_RUNNING on an asynchronous
if (rpc_test_and_set_running(task)) * rpc_task, rpc_make_runnable() can assign it to a
return; * different workqueue. We therefore cannot assume that the
* rpc_task pointer may still be dereferenced.
*/
queue = task->tk_waitqueue;
spin_lock_bh(&queue->lock);
if (!RPC_IS_QUEUED(task)) {
spin_unlock_bh(&queue->lock);
continue; continue;
} }
rpc_clear_running(task);
spin_unlock_bh(&queue->lock);
if (task_is_async)
return;
/* sync task: sleep here */ /* sync task: sleep here */
dprintk("RPC: %5u sync task going to sleep\n", task->tk_pid); dprintk("RPC: %5u sync task going to sleep\n", task->tk_pid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment