* [PATCH] Do not enter setup_callback_client when finding backchannel failed @ 2013-11-27 10:29 Kinglong Mee 2013-12-02 14:59 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Kinglong Mee @ 2013-11-27 10:29 UTC (permalink / raw) To: bfields, linux-nfs If finding backchannel failed, nfsd should not enter setup_callback_client. Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> --- 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) 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; if (err) { nfsd4_mark_cb_down(clp, err); return; -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-11-27 10:29 [PATCH] Do not enter setup_callback_client when finding backchannel failed Kinglong Mee @ 2013-12-02 14:59 ` J. Bruce Fields 2013-12-03 2:46 ` Kinglong Mee 2013-12-03 3:40 ` [PATCH v2] " Kinglong Mee 0 siblings, 2 replies; 8+ messages in thread From: J. Bruce Fields @ 2013-12-02 14:59 UTC (permalink / raw) To: Kinglong Mee; +Cc: linux-nfs 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@gmail.com> > --- > 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? > 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?) But I think we should probably adopt the sort of control flow that's more idiomatic in the kernel: c = __nfsd4_find_backchannel(clp) if (!c) goto out_unlock; ... spin_unlock(&clp->cl_lock); err = setup_callback_client(clp, &conn, ses); if (err) goto out_err; etc. --b. > if (err) { > nfsd4_mark_cb_down(clp, err); > return; > -- > 1.8.4.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-12-02 14:59 ` J. Bruce Fields @ 2013-12-03 2:46 ` Kinglong Mee 2013-12-04 17:10 ` J. Bruce Fields 2013-12-03 3:40 ` [PATCH v2] " Kinglong Mee 1 sibling, 1 reply; 8+ messages in thread From: Kinglong Mee @ 2013-12-03 2:46 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs 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@gmail.com> >> --- >> 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. (Ps: Maybe needs another patch to avoid updating callback connect when destroying the last session ?) For debugging, add message when finding backchannel failed. Just testing mount/umount at client, the debug message will be print every umount. diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 7f05cd1..1be4c61 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -979,6 +979,8 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb) svc_xprt_get(c->cn_xprt); conn.cb_xprt = c->cn_xprt; ses = c->cn_session; + } else { + printk("%s: found connet failed.\n", __func__); } spin_unlock(&clp->cl_lock); > But I think we should probably adopt the sort of > control flow that's more idiomatic in the kernel: > > c = __nfsd4_find_backchannel(clp) > if (!c) > goto out_unlock; > ... > spin_unlock(&clp->cl_lock); > err = setup_callback_client(clp, &conn, ses); > if (err) > goto out_err; > OK, it is really better. thanks, Kinglong Mee > etc. > > --b. > >> if (err) { >> nfsd4_mark_cb_down(clp, err); >> return; >> -- >> 1.8.4.2 > ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-12-03 2:46 ` Kinglong Mee @ 2013-12-04 17:10 ` J. Bruce Fields 2013-12-05 2:09 ` Kinglong Mee 0 siblings, 1 reply; 8+ messages in thread From: J. Bruce Fields @ 2013-12-04 17:10 UTC (permalink / raw) To: Kinglong Mee; +Cc: linux-nfs 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@gmail.com> > >> --- > >> 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. Is that the only problem? --b. > (Ps: Maybe needs > another patch to avoid updating callback connect when destroying the last session ?) > > For debugging, add message when finding backchannel failed. > Just testing mount/umount at client, the debug message will be print every umount. > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index 7f05cd1..1be4c61 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -979,6 +979,8 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb) > svc_xprt_get(c->cn_xprt); > conn.cb_xprt = c->cn_xprt; > ses = c->cn_session; > + } else { > + printk("%s: found connet failed.\n", __func__); > } > spin_unlock(&clp->cl_lock); > > > But I think we should probably adopt the sort of > > control flow that's more idiomatic in the kernel: > > > > c = __nfsd4_find_backchannel(clp) > > if (!c) > > goto out_unlock; > > ... > > spin_unlock(&clp->cl_lock); > > err = setup_callback_client(clp, &conn, ses); > > if (err) > > goto out_err; > > > > OK, it is really better. > > thanks, > Kinglong Mee > > > etc. > > > > --b. > > > >> if (err) { > >> nfsd4_mark_cb_down(clp, err); > >> return; > >> -- > >> 1.8.4.2 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-12-04 17:10 ` J. Bruce Fields @ 2013-12-05 2:09 ` Kinglong Mee 2013-12-06 18:00 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Kinglong Mee @ 2013-12-05 2:09 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs 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@gmail.com> >>>> --- >>>> 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. thanks, Kinglong Mee >> (Ps: Maybe needs >> another patch to avoid updating callback connect when destroying the last session ?) >> >> For debugging, add message when finding backchannel failed. >> Just testing mount/umount at client, the debug message will be print every umount. >> >> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c >> index 7f05cd1..1be4c61 100644 >> --- a/fs/nfsd/nfs4callback.c >> +++ b/fs/nfsd/nfs4callback.c >> @@ -979,6 +979,8 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb) >> svc_xprt_get(c->cn_xprt); >> conn.cb_xprt = c->cn_xprt; >> ses = c->cn_session; >> + } else { >> + printk("%s: found connet failed.\n", __func__); >> } >> spin_unlock(&clp->cl_lock); >> >>> But I think we should probably adopt the sort of >>> control flow that's more idiomatic in the kernel: >>> >>> c = __nfsd4_find_backchannel(clp) >>> if (!c) >>> goto out_unlock; >>> ... >>> spin_unlock(&clp->cl_lock); >>> err = setup_callback_client(clp, &conn, ses); >>> if (err) >>> goto out_err; >>> >> >> OK, it is really better. >> >> thanks, >> Kinglong Mee >> >>> etc. >>> >>> --b. >>> >>>> if (err) { >>>> nfsd4_mark_cb_down(clp, err); >>>> return; >>>> -- >>>> 1.8.4.2 >>> >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-12-05 2:09 ` Kinglong Mee @ 2013-12-06 18:00 ` J. Bruce Fields 2013-12-09 1:17 ` Kinglong Mee 0 siblings, 1 reply; 8+ messages in thread From: J. Bruce Fields @ 2013-12-06 18:00 UTC (permalink / raw) To: Kinglong Mee; +Cc: linux-nfs 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@gmail.com> > >>>> --- > >>>> 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? --b. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Do not enter setup_callback_client when finding backchannel failed 2013-12-06 18:00 ` J. Bruce Fields @ 2013-12-09 1:17 ` Kinglong Mee 0 siblings, 0 replies; 8+ messages in thread From: Kinglong Mee @ 2013-12-09 1:17 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs 2013/12/7 J. Bruce Fields <bfields@fieldses.org>: > 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@gmail.com> >> >>>> --- >> >>>> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] Do not enter setup_callback_client when finding backchannel failed 2013-12-02 14:59 ` J. Bruce Fields 2013-12-03 2:46 ` Kinglong Mee @ 2013-12-03 3:40 ` Kinglong Mee 1 sibling, 0 replies; 8+ messages in thread From: Kinglong Mee @ 2013-12-03 3:40 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs Finding backchannel failed, nfsd should not enter setup_callback_client. v2. drop ses checking in setup_callback_client and make code more clearer. Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> --- fs/nfsd/nfs4callback.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 7f05cd1..44d33ce 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -975,11 +975,15 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb) clear_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags); memcpy(&conn, &cb->cb_clp->cl_cb_conn, sizeof(struct nfs4_cb_conn)); c = __nfsd4_find_backchannel(clp); - if (c) { - svc_xprt_get(c->cn_xprt); - conn.cb_xprt = c->cn_xprt; - ses = c->cn_session; + if (!c) { + spin_unlock(&clp->cl_lock); + nfsd4_mark_cb_down(clp, -ENOTCONN); + return; } + + svc_xprt_get(c->cn_xprt); + conn.cb_xprt = c->cn_xprt; + ses = c->cn_session; spin_unlock(&clp->cl_lock); err = setup_callback_client(clp, &conn, ses); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-12-09 1:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-27 10:29 [PATCH] Do not enter setup_callback_client when finding backchannel failed Kinglong Mee 2013-12-02 14:59 ` J. Bruce Fields 2013-12-03 2:46 ` Kinglong Mee 2013-12-04 17:10 ` J. Bruce Fields 2013-12-05 2:09 ` Kinglong Mee 2013-12-06 18:00 ` J. Bruce Fields 2013-12-09 1:17 ` Kinglong Mee 2013-12-03 3:40 ` [PATCH v2] " Kinglong Mee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).