From: Kinglong Mee <kinglongmee@gmail.com>
To: Dr Fields James Bruce <bfields@fieldses.org>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>,
Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] SUNRPC: Clear xpt_bc_xprt if xs_setup_bc_tcp failed
Date: Fri, 10 Jan 2014 11:22:37 +0800 [thread overview]
Message-ID: <52CF677D.80803@gmail.com> (raw)
In-Reply-To: <52CF5E61.5040002@gmail.com>
On 01/10/2014 10:43 AM, Kinglong Mee wrote:
> On 01/10/2014 12:34 AM, Dr Fields James Bruce wrote:
>> On Tue, Jan 07, 2014 at 03:26:04PM +0800, Kinglong Mee wrote:
>>> On 01/07/2014 01:01 PM, Trond Myklebust wrote:
>>>>
>>>> On Jan 6, 2014, at 23:41, Kinglong Mee <kinglongmee@gmail.com> wrote:
>>>>
>>>>> If try_module_get failed, xpt_bc_xprt should be set to NULL,
>>>>> because xprt will be free.
>>>>>
>>>>> Don't needed using xprt_put to free xprt, because it is always new.
>>>>>
>>>>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>>>>> ---
>>>>> net/sunrpc/xprtsock.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>>>>> index 4fcdf74..5ed124f 100644
>>>>> --- a/net/sunrpc/xprtsock.c
>>>>> +++ b/net/sunrpc/xprtsock.c
>>>>> @@ -2986,10 +2986,10 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
>>>>> */
>>>>> xprt_set_connected(xprt);
>>>>>
>>>>> -
>>>>> if (try_module_get(THIS_MODULE))
>>>>> return xprt;
>>>>> - xprt_put(xprt);
>>>>> +
>>>>> + args->bc_xprt->xpt_bc_xprt = NULL;
>>>>> ret = ERR_PTR(-EINVAL);
>>>>> out_err:
>>>>> xs_xprt_free(xprt);
>>>>
>>>> Just move the 'args->bc_xprt->xpt_bc_xprt = xprt’ line into the caller (see earlier discussion about xs_setup_bc_tcp()). Quite frankly, I don’t see why the client code should be modifying this variable in the first place. An svc_xprt is a server construct.
>>>
>>> Hi Bruce,
>>>
>>> When reviewing those codes for bc_xprt, I found commit d75faea330dbd1873c9094e9926ae306590c0998
>>> 'rpc: move sk_bc_xprt to svc_xprt'. I'd like recording bc_xprt in svc_sock.
>>
>> OK, I think it would be alright to revert that commit.
>
> I will try to revert it later.
I had try to revert it, but, I don't revert it.
I'm sorry for my above thinking.
I think it is not needed to move bc_xprt from one place to another
without fix some problem, because knfsd running correctly with that.
Just let bc_xprt in svc_xprt until some problem appears.
Thanks,
Kinglong Mee
prev parent reply other threads:[~2014-01-10 3:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-07 4:41 [PATCH] SUNRPC: Clear xpt_bc_xprt if xs_setup_bc_tcp failed Kinglong Mee
2014-01-07 5:01 ` Trond Myklebust
2014-01-07 5:21 ` Kinglong Mee
2014-01-07 7:26 ` Kinglong Mee
2014-01-09 16:34 ` Dr Fields James Bruce
2014-01-10 2:43 ` Kinglong Mee
2014-01-10 3:22 ` Kinglong Mee [this message]
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=52CF677D.80803@gmail.com \
--to=kinglongmee@gmail.com \
--cc=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@primarydata.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.