* [PATCH 03/27] SUNRPC: Check a return result @ 2007-10-26 17:30 Chuck Lever 2007-10-30 16:42 ` James Lentini 0 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2007-10-26 17:30 UTC (permalink / raw) To: trond.myklebust; +Cc: nfs Minor: Replace an empty if statement with a debugging dprintk. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: Thomas Talpey <Thomas.Talpey@netapp.com> --- net/sunrpc/xprtrdma/verbs.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 44b0fb9..ffbf22a 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -522,7 +522,7 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, struct rpcrdma_create_data_internal *cdata) { struct ib_device_attr devattr; - int rc; + int rc, err; rc = ib_query_device(ia->ri_id->device, &devattr); if (rc) { @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, return 0; out2: - if (ib_destroy_cq(ep->rep_cq)) - ; + err = ib_destroy_cq(ep->rep_cq); + if (err) + dprintk("RPC: %s: ib_destroy_cq returned %i\n", + __func__, err); out1: return rc; } ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 03/27] SUNRPC: Check a return result 2007-10-26 17:30 [PATCH 03/27] SUNRPC: Check a return result Chuck Lever @ 2007-10-30 16:42 ` James Lentini 2007-10-30 16:49 ` Chuck Lever 2007-10-30 16:56 ` Trond Myklebust 0 siblings, 2 replies; 6+ messages in thread From: James Lentini @ 2007-10-30 16:42 UTC (permalink / raw) To: Chuck Lever, trond.myklebust; +Cc: nfs On Fri, 26 Oct 2007, Chuck Lever wrote: > Minor: Replace an empty if statement with a debugging dprintk. > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > Cc: Thomas Talpey <Thomas.Talpey@netapp.com> > --- > > net/sunrpc/xprtrdma/verbs.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > index 44b0fb9..ffbf22a 100644 > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -522,7 +522,7 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, > struct rpcrdma_create_data_internal *cdata) > { > struct ib_device_attr devattr; > - int rc; > + int rc, err; > > rc = ib_query_device(ia->ri_id->device, &devattr); > if (rc) { > @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, > return 0; > > out2: > - if (ib_destroy_cq(ep->rep_cq)) > - ; > + err = ib_destroy_cq(ep->rep_cq); > + if (err) > + dprintk("RPC: %s: ib_destroy_cq returned %i\n", > + __func__, err); > out1: > return rc; > } Good eyes, Chuck. One minor suggestion: instead of adding "err", how about reusing "rc"? Here's a patch against Trond's nfs-2.6 tree. Signed-off-by: James Lentini <jlentini@netapp.com> verbs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, return 0; out2: - if (ib_destroy_cq(ep->rep_cq)) - ; + rc = ib_destroy_cq(ep->rep_cq); + if (rc) + dprintk("RPC: %s: ib_destroy_cq returned %i\n", + __func__, rc); out1: return rc; } ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 03/27] SUNRPC: Check a return result 2007-10-30 16:42 ` James Lentini @ 2007-10-30 16:49 ` Chuck Lever 2007-10-30 16:59 ` Talpey, Thomas 2007-10-30 16:56 ` Trond Myklebust 1 sibling, 1 reply; 6+ messages in thread From: Chuck Lever @ 2007-10-30 16:49 UTC (permalink / raw) To: James Lentini; +Cc: nfs, trond.myklebust [-- Attachment #1: Type: text/plain, Size: 2048 bytes --] James Lentini wrote: > On Fri, 26 Oct 2007, Chuck Lever wrote: > >> Minor: Replace an empty if statement with a debugging dprintk. >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> Cc: Thomas Talpey <Thomas.Talpey@netapp.com> >> --- >> >> net/sunrpc/xprtrdma/verbs.c | 8 +++++--- >> 1 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c >> index 44b0fb9..ffbf22a 100644 >> --- a/net/sunrpc/xprtrdma/verbs.c >> +++ b/net/sunrpc/xprtrdma/verbs.c >> @@ -522,7 +522,7 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, >> struct rpcrdma_create_data_internal *cdata) >> { >> struct ib_device_attr devattr; >> - int rc; >> + int rc, err; >> >> rc = ib_query_device(ia->ri_id->device, &devattr); >> if (rc) { >> @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, >> return 0; >> >> out2: >> - if (ib_destroy_cq(ep->rep_cq)) >> - ; >> + err = ib_destroy_cq(ep->rep_cq); >> + if (err) >> + dprintk("RPC: %s: ib_destroy_cq returned %i\n", >> + __func__, err); >> out1: >> return rc; >> } > > Good eyes, Chuck. One minor suggestion: instead of adding "err", how > about reusing "rc"? Here's a patch against Trond's nfs-2.6 tree. I added "err" because it appeared that the intention of the original logic was to ignore the return code from ib_destroy_cq() completely. Thus re-using "rc" would have made rpcrdma_ep_create() return an unnecessary error in cases where it hadn't before. > Signed-off-by: James Lentini <jlentini@netapp.com> > > verbs.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, > return 0; > > out2: > - if (ib_destroy_cq(ep->rep_cq)) > - ; > + rc = ib_destroy_cq(ep->rep_cq); > + if (rc) > + dprintk("RPC: %s: ib_destroy_cq returned %i\n", > + __func__, rc); > out1: > return rc; > } [-- Attachment #2: chuck.lever.vcf --] [-- Type: text/x-vcard, Size: 259 bytes --] begin:vcard fn:Chuck Lever n:Lever;Chuck org:Oracle Corporation;Corporate Architecture: Linux Projects Group adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA title:Principal Member of Staff tel;work:+1 248 614 5091 x-mozilla-html:FALSE version:2.1 end:vcard [-- Attachment #3: Type: text/plain, Size: 314 bytes --] ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ [-- Attachment #4: Type: text/plain, Size: 140 bytes --] _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 03/27] SUNRPC: Check a return result 2007-10-30 16:49 ` Chuck Lever @ 2007-10-30 16:59 ` Talpey, Thomas 2007-10-30 17:09 ` James Lentini 0 siblings, 1 reply; 6+ messages in thread From: Talpey, Thomas @ 2007-10-30 16:59 UTC (permalink / raw) To: chuck.lever; +Cc: nfs, trond.myklebust At 12:49 PM 10/30/2007, Chuck Lever wrote: >I added "err" because it appeared that the intention of the original >logic was to ignore the return code from ib_destroy_cq() completely. >Thus re-using "rc" would have made rpcrdma_ep_create() return an >unnecessary error in cases where it hadn't before. Which would be bad, mounts would fail for no reason. Your fix is correct - though really all the code needs is (void). Unfortunately when I coded that, some tool whined, I forget which. Anyway, dprintk'ing the message is better. Tom. > >> Signed-off-by: James Lentini <jlentini@netapp.com> >> >> verbs.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> --- a/net/sunrpc/xprtrdma/verbs.c >> +++ b/net/sunrpc/xprtrdma/verbs.c >> @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, >> return 0; >> >> out2: >> - if (ib_destroy_cq(ep->rep_cq)) >> - ; >> + rc = ib_destroy_cq(ep->rep_cq); >> + if (rc) >> + dprintk("RPC: %s: ib_destroy_cq returned %i\n", >> + __func__, rc); >> out1: >> return rc; >> } > > >------------------------------------------------------------------------- >This SF.net email is sponsored by: Splunk Inc. >Still grepping through log files to find problems? Stop. >Now Search log events and configuration files using AJAX and a browser. >Download your FREE copy of Splunk now >> http://get.splunk.com/ >_______________________________________________ >NFS maillist - NFS@lists.sourceforge.net >https://lists.sourceforge.net/lists/listinfo/nfs ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 03/27] SUNRPC: Check a return result 2007-10-30 16:59 ` Talpey, Thomas @ 2007-10-30 17:09 ` James Lentini 0 siblings, 0 replies; 6+ messages in thread From: James Lentini @ 2007-10-30 17:09 UTC (permalink / raw) To: Talpey, Thomas; +Cc: nfs, trond.myklebust On Tue, 30 Oct 2007, Talpey, Thomas wrote: > At 12:49 PM 10/30/2007, Chuck Lever wrote: > >I added "err" because it appeared that the intention of the original > >logic was to ignore the return code from ib_destroy_cq() completely. > >Thus re-using "rc" would have made rpcrdma_ep_create() return an > >unnecessary error in cases where it hadn't before. > > Which would be bad, mounts would fail for no reason. Your fix is > correct - though really all the code needs is (void). Unfortunately > when I coded that, some tool whined, I forget which. Anyway, > dprintk'ing the message is better. Good point. I missed that. > > > >> Signed-off-by: James Lentini <jlentini@netapp.com> > >> verbs.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> --- a/net/sunrpc/xprtrdma/verbs.c > >> +++ b/net/sunrpc/xprtrdma/verbs.c > >> @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, > >> return 0; > >> > >> out2: > >> - if (ib_destroy_cq(ep->rep_cq)) > >> - ; > >> + rc = ib_destroy_cq(ep->rep_cq); > >> + if (rc) > >> + dprintk("RPC: %s: ib_destroy_cq returned %i\n", > >> + __func__, rc); > >> out1: > >> return rc; > >> } > > > > > >------------------------------------------------------------------------- > >This SF.net email is sponsored by: Splunk Inc. > >Still grepping through log files to find problems? Stop. > >Now Search log events and configuration files using AJAX and a browser. > >Download your FREE copy of Splunk now >> http://get.splunk.com/ > >_______________________________________________ > >NFS maillist - NFS@lists.sourceforge.net > >https://lists.sourceforge.net/lists/listinfo/nfs > > ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 03/27] SUNRPC: Check a return result 2007-10-30 16:42 ` James Lentini 2007-10-30 16:49 ` Chuck Lever @ 2007-10-30 16:56 ` Trond Myklebust 1 sibling, 0 replies; 6+ messages in thread From: Trond Myklebust @ 2007-10-30 16:56 UTC (permalink / raw) To: James Lentini; +Cc: nfs On Tue, 2007-10-30 at 12:42 -0400, James Lentini wrote: > On Fri, 26 Oct 2007, Chuck Lever wrote: > > > Minor: Replace an empty if statement with a debugging dprintk. > > > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > > Cc: Thomas Talpey <Thomas.Talpey@netapp.com> > > --- > > > > net/sunrpc/xprtrdma/verbs.c | 8 +++++--- > > 1 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > > index 44b0fb9..ffbf22a 100644 > > --- a/net/sunrpc/xprtrdma/verbs.c > > +++ b/net/sunrpc/xprtrdma/verbs.c > > @@ -522,7 +522,7 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, > > struct rpcrdma_create_data_internal *cdata) > > { > > struct ib_device_attr devattr; > > - int rc; > > + int rc, err; > > > > rc = ib_query_device(ia->ri_id->device, &devattr); > > if (rc) { > > @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, struct rpcrdma_ia *ia, > > return 0; > > > > out2: > > - if (ib_destroy_cq(ep->rep_cq)) > > - ; > > + err = ib_destroy_cq(ep->rep_cq); > > + if (err) > > + dprintk("RPC: %s: ib_destroy_cq returned %i\n", > > + __func__, err); > > out1: > > return rc; > > } > > Good eyes, Chuck. One minor suggestion: instead of adding "err", how > about reusing "rc"? Here's a patch against Trond's nfs-2.6 tree. > > Signed-off-by: James Lentini <jlentini@netapp.com> > > verbs.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -648,8 +648,10 @@ rpcrdma_ep_create(struct rpcrdma_ep *ep, > return 0; > > out2: > - if (ib_destroy_cq(ep->rep_cq)) > - ; > + rc = ib_destroy_cq(ep->rep_cq); > + if (rc) > + dprintk("RPC: %s: ib_destroy_cq returned %i\n", > + __func__, rc); > out1: > return rc; > } NAK. That would clobber the error that you are trying to return from ib_req_notify_cq. Cheers Trond ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-30 17:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-26 17:30 [PATCH 03/27] SUNRPC: Check a return result Chuck Lever 2007-10-30 16:42 ` James Lentini 2007-10-30 16:49 ` Chuck Lever 2007-10-30 16:59 ` Talpey, Thomas 2007-10-30 17:09 ` James Lentini 2007-10-30 16:56 ` Trond Myklebust
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.