| Message ID | 162066193457.94415.10829735588517134118.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD callback and lease management observability | expand |
On Mon, May 10, 2021 at 11:52:14AM -0400, Chuck Lever wrote: > This path is not really an error path, What's the non-error case for this path? On a quick look it seems like that'd mean a 4.1 client doesn't have a connection available for the backchannel, which sounds bad. But I'm probably overlooking something.... --b. > so the tracepoint I added > there is just noise. > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfsd/nfs4callback.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index ab1836381e22..15ba16c54793 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -915,10 +915,8 @@ 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) { > - trace_nfsd_cb_setup_err(clp, -EINVAL); > + if (!conn->cb_xprt) > return -EINVAL; > - } > clp->cl_cb_conn.cb_xprt = conn->cb_xprt; > clp->cl_cb_session = ses; > args.bc_xprt = conn->cb_xprt; >
> On May 10, 2021, at 4:27 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Mon, May 10, 2021 at 11:52:14AM -0400, Chuck Lever wrote: >> This path is not really an error path, > > What's the non-error case for this path? From what I can tell, it appears to be the default exit for when there is a session and backchannel. Feel free to straighten me out, but it just seemed to always fire for NFSv4.1 mounts. > On a quick look it seems like that'd mean a 4.1 client doesn't have a > connection available for the backchannel, which sounds bad. > > But I'm probably overlooking something.... > > --b. > >> so the tracepoint I added >> there is just noise. >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> fs/nfsd/nfs4callback.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c >> index ab1836381e22..15ba16c54793 100644 >> --- a/fs/nfsd/nfs4callback.c >> +++ b/fs/nfsd/nfs4callback.c >> @@ -915,10 +915,8 @@ 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) { >> - trace_nfsd_cb_setup_err(clp, -EINVAL); >> + if (!conn->cb_xprt) >> return -EINVAL; >> - } >> clp->cl_cb_conn.cb_xprt = conn->cb_xprt; >> clp->cl_cb_session = ses; >> args.bc_xprt = conn->cb_xprt; >> -- Chuck Lever
On Mon, May 10, 2021 at 08:29:32PM +0000, Chuck Lever III wrote: > > > > On May 10, 2021, at 4:27 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > > > On Mon, May 10, 2021 at 11:52:14AM -0400, Chuck Lever wrote: > >> This path is not really an error path, > > > > What's the non-error case for this path? > > >From what I can tell, it appears to be the default exit for when > there is a session and backchannel. Feel free to straighten me > out, but it just seemed to always fire for NFSv4.1 mounts. I'd be curious to know why. I'll see if I can find some time to experiment. --b. > > On a quick look it seems like that'd mean a 4.1 client doesn't have a > > connection available for the backchannel, which sounds bad. > > > > But I'm probably overlooking something.... > > > > --b. > > > >> so the tracepoint I added > >> there is just noise. > >> > >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > >> --- > >> fs/nfsd/nfs4callback.c | 4 +--- > >> 1 file changed, 1 insertion(+), 3 deletions(-) > >> > >> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > >> index ab1836381e22..15ba16c54793 100644 > >> --- a/fs/nfsd/nfs4callback.c > >> +++ b/fs/nfsd/nfs4callback.c > >> @@ -915,10 +915,8 @@ 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) { > >> - trace_nfsd_cb_setup_err(clp, -EINVAL); > >> + if (!conn->cb_xprt) > >> return -EINVAL; > >> - } > >> clp->cl_cb_conn.cb_xprt = conn->cb_xprt; > >> clp->cl_cb_session = ses; > >> args.bc_xprt = conn->cb_xprt; > >> > > -- > Chuck Lever > >
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index ab1836381e22..15ba16c54793 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -915,10 +915,8 @@ 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) { - trace_nfsd_cb_setup_err(clp, -EINVAL); + if (!conn->cb_xprt) return -EINVAL; - } clp->cl_cb_conn.cb_xprt = conn->cb_xprt; clp->cl_cb_session = ses; args.bc_xprt = conn->cb_xprt;
This path is not really an error path, so the tracepoint I added there is just noise. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfsd/nfs4callback.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)