2013/12/7 J. Bruce Fields <bfields@xxxxxxxxxxxx>:
> On Thu, Dec 05, 2013 at 10:09:52AM +0800, Kinglong Mee wrote:
>> On 12/05/2013 01:10 AM, J. Bruce Fields wrote:
>> > On Tue, Dec 03, 2013 at 10:46:47AM +0800, Kinglong Mee wrote:
>> >> On 12/02/2013 10:59 PM, J. Bruce Fields wrote:
>> >>> On Wed, Nov 27, 2013 at 06:29:22PM +0800, Kinglong Mee wrote:
>> >>>> If finding backchannel failed, nfsd should not enter setup_callback_client.
>> >>>>
>> >>>> Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
>> >>>> ---
>> >>>> fs/nfsd/nfs4callback.c | 4 ++--
>> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>> >>>> index 7f05cd1..755d6d6 100644
>> >>>> --- a/fs/nfsd/nfs4callback.c
>> >>>> +++ b/fs/nfsd/nfs4callback.c
>> >>>> @@ -664,7 +664,7 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
>> >>>> args.authflavor = clp->cl_cred.cr_flavor;
>> >>>> clp->cl_cb_ident = conn->cb_ident;
>> >>>> } else {
>> >>>> - if (!conn->cb_xprt)
>> >>>> + if (!conn->cb_xprt || !ses)
>> >>>
>> >>> It looks to me like ses should be set whenenver conn is. Do you have
>> >>> reason to believe the contrary?
>> >>
>> >> No. It just a double check as conn->cb_xprt.
>> >> I will drop it in v2 of this path.
>> >>
>> >>>
>> >>>> return -EINVAL;
>> >>>> clp->cl_cb_conn.cb_xprt = conn->cb_xprt;
>> >>>> clp->cl_cb_session = ses;
>> >>>> @@ -982,7 +982,7 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
>> >>>> }
>> >>>> spin_unlock(&clp->cl_lock);
>> >>>>
>> >>>> - err = setup_callback_client(clp, &conn, ses);
>> >>>> + err = c ? setup_callback_client(clp, &conn, ses) : -ENOENT;
>> >>>
>> >>> But, whoops, yes, this looks like a good fix. (Have you hit this
>> >>> failure in practice?)
>> >>
>> >> Yes. When destroying the last session from client, nfsd4_probe_callback_sync
>> >> will be called in function nfsd4_destroy_session.
>> >>
>> >> As that, callback client will be update in function nfsd4_process_cb_update,
>> >> so __nfsd4_find_backchannel always failed with returning NULL.
>> >
>> > On second thoughts, why is that actually a problem?
>> > setup_callback_client() will fail and you'll get an unnecessary printk
>> > from nfsd4_mark_cb_down, I guess.
>>
>> Yes, that's right. setup_callback_client will failed with －EINVAL.
>> This patch just changes the logic, function returns immediately when finding backchannel failed,
>> instead of enter setup_callback_client.
>>
>> > Is that the only problem?
>>
>> Yes, this path only focus the logic problem.
>
> Apologies, I'm still not sure I understand:
>
> As far as I can tell, the *only* way that your patch changes behavior is
> to change the error passed to nfsd4_mark_cb_path down.
>
> Is that right, or am I missing some other problem which this fixes?
Sorry for the not clearly explan.
there are two behavior of this patch.
the first is as you said, change the error number.
the second is the logic, return immediately when finding session
failed, not enter setup_callback_client.
thanks,
Kinglong Mee
--
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
