From: Stanislav Kinsbursky <skinsbursky@parallels•com>
To: Trond Myklebust <Trond.Myklebust@netapp•com>
Cc: "linux-nfs@vger•kernel.org" <linux-nfs@vger•kernel.org>,
Pavel Emelianov <xemul@parallels•com>,
"neilb@suse•de" <neilb@suse•de>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"linux-kernel@vger•kernel.org" <linux-kernel@vger•kernel.org>,
"bfields@fieldses•org" <bfields@fieldses•org>,
"davem@davemloft•net" <davem@davemloft•net>,
"skinsbursky@openvz•org" <skinsbursky@openvz•org>
Subject: Re: [PATCH] RPC: killing RPC tasks races fixed
Date: Thu, 17 Mar 2011 18:43:56 +0300 [thread overview]
Message-ID: <4D822C3C.9090100@parallels.com> (raw)
In-Reply-To: <1300366904.9654.13.camel@lade.trondhjem.org>
17.03.2011 16:01, Trond Myklebust пишет:
> On Thu, 2011-03-17 at 15:16 +0300, Stanislav Kinsbursky wrote:
>> task->tk_waitqueue must be checked for NULL before trying to wake up task in
>> rpc_killall_tasks() because it can be NULL.
>>
>> Here is an example:
>>
>> CPU 0 CPU 1 CPU 2
>> -------------------- --------------------- --------------------------
>> nfs4_run_open_task
>> rpc_run_task
>> rpc_execute
>> rpc_set_active
>> rpc_make_runnable
>> (waiting)
>> rpc_async_schedule
>> nfs4_open_prepare
>> nfs_wait_on_sequence
>> nfs_umount_begin
>> rpc_killall_tasks
>> rpc_wake_up_task
>> rpc_wake_up_queued_task
>> spin_lock(tk_waitqueue == NULL)
>> BUG()
>> rpc_sleep_on
>> spin_lock(&q->lock)
>> __rpc_sleep_on
>> task->tk_waitqueue = q
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky@openvz•org>
>>
>> ---
>> net/sunrpc/clnt.c | 4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
>> index 57d344c..24039fe 100644
>> --- a/net/sunrpc/clnt.c
>> +++ b/net/sunrpc/clnt.c
>> @@ -436,7 +436,9 @@ void rpc_killall_tasks(struct rpc_clnt *clnt)
>> if (!(rovr->tk_flags& RPC_TASK_KILLED)) {
>> rovr->tk_flags |= RPC_TASK_KILLED;
>> rpc_exit(rovr, -EIO);
>> - rpc_wake_up_queued_task(rovr->tk_waitqueue, rovr);
>> + if (rovr->tk_waitqueue)
>> + rpc_wake_up_queued_task(rovr->tk_waitqueue,
>> + rovr);
>
> Testing for RPC_IS_QUEUED(rovr) would be better, since that would
> optimise away the call to rpc_wake_up_queued_task() altogether for those
> tasks that aren't queued.
>
Yes, I agree with testing RPC_IS_QUEUED(rovr) since such approach looks
clearer and in 2.6.38 tk_waitqueue is initialized prior to set
RPC_TASK_QUEUED bit.
But I found this problem in 2.6.32 rhel kernel where this set sequence is inversed.
Will send fixed version soon.
>> }
>> }
>> spin_unlock(&clnt->cl_lock);
>>
>
--
Best regards,
Stanislav Kinsbursky
next prev parent reply other threads:[~2011-03-17 15:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-17 12:16 [PATCH] RPC: killing RPC tasks races fixed Stanislav Kinsbursky
[not found] ` <20110317121638.15035.39410.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>
2011-03-17 13:01 ` Trond Myklebust
2011-03-17 15:43 ` Stanislav Kinsbursky [this message]
[not found] ` <4D822C3C.9090100-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
2011-03-17 16:44 ` Trond Myklebust
2011-03-17 17:04 ` Stanislav Kinsbursky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D822C3C.9090100@parallels.com \
--to=skinsbursky@parallels$(echo .)com \
--cc=Trond.Myklebust@netapp$(echo .)com \
--cc=bfields@fieldses$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-nfs@vger$(echo .)kernel.org \
--cc=neilb@suse$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=skinsbursky@openvz$(echo .)org \
--cc=xemul@parallels$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox