[PATCH 3/3] Return errors in bc_send

[Andy Adamson]

Release the request and task upon allocation errors.

Don't just return zero, return the send_request return value.
---
 net/sunrpc/bc_svc.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/bc_svc.c b/net/sunrpc/bc_svc.c
index c713f05..a9be1b4 100644
--- a/net/sunrpc/bc_svc.c
+++ b/net/sunrpc/bc_svc.c
@@ -131,7 +131,7 @@ void bc_release_xprt(struct rpc_task *task)
 int bc_send(struct rpc_rqst *req)
 {
        struct rpc_task *bc_task;
-       int err;
+       int err = -ENOMEM;
        struct xdr_buf *xbufp = &req->rq_snd_buf;
        struct rpc_xprt *xprt = req->rq_xprt;

@@ -139,13 +139,15 @@ int bc_send(struct rpc_rqst *req)
         * Create an rpc_task to send the data
         */
        bc_task = bc_alloc_task(req);
+       if (!bc_task)
+               goto out_free_req;

        /*
         * Reserve the xprt and then try to send the request across
         */
        err = bc_reserve_xprt(bc_task);
        if (err)
-               goto out;
+               goto out_free_task;

        /*
         * Set up the xdr_buf length
@@ -156,12 +158,14 @@ int bc_send(struct rpc_rqst *req)

        bc_release_xprt(bc_task);

+out_free_req:
        xprt_free_bc_request(req);
-       return 0;
-out:
+       return err;
+
+out_free_task:
        bc_task->tk_rqstp = NULL;
        bc_release_task(bc_task);
-       return err;
+       goto out_free_req;
 }

 /*
-- 




On 5/24/07, Iyer, Rahul <Rahul.Iyer at netapp.com> wrote:
>
>
> My bad. I see it now.
> Regards
> Rahul
>
> > -----Original Message-----
> > From: Benny Halevy [mailto:bhalevy at panasas.com]
> > Sent: Thursday, May 24, 2007 9:07 AM
> > To: undisclosed-recipients
> > Subject: [pnfs] [PATCH 2/2] bc_send: handle error from bc_alloc_task
> >
> > >From 3ad9cc52abc6146a4e0714b797feb3473d829e56 Mon Sep 17
> > 00:00:00 2001
> > From: Benny Halevy <bhalevy at panasas.com>
> > Date: Thu, 24 May 2007 19:07:13 +0300
> > Subject: [PATCH 2/2] bc_send: handle error from bc_alloc_task
> >
> > Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> > ---
> >  net/sunrpc/bc_svc.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/sunrpc/bc_svc.c b/net/sunrpc/bc_svc.c
> > index fbea284..a8519d5 100644
> > --- a/net/sunrpc/bc_svc.c
> > +++ b/net/sunrpc/bc_svc.c
> > @@ -139,6 +139,10 @@ int bc_send(struct rpc_rqst *req)
> >        * Create an rpc_task to send the data
> >        */
> >       bc_task = bc_alloc_task(req);
> > +     if (!bc_task) {
> > +             err = -ENOMEM;
> > +             goto out_no_free;
> > +     }
> >
> >       /*
> >        * Reserve the xprt and then try to send the request across
> > @@ -158,6 +162,7 @@ int bc_send(struct rpc_rqst *req)
> >  out:
> >       bc_task->tk_rqstp = NULL;
> >       bc_release_task(bc_task);
> > +out_no_free:
> >       xprt_free_bc_request(req);
> >       return err;
> >  }
> > --
> > 1.5.1
> >
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>

------=_Part_128865_29770799.1180111924724
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

This patch did not apply to the 4.1-sessions branch. There is no xprt_free_bc_request(req); after the out: label.<br>
<br>
here is the function in the 4.1-sessions branch.<br>
<br>
/*<br>
&nbsp;* Send the callback reply<br>
&nbsp;*/<br>
int bc_send(struct rpc_rqst *req)<br>
{<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; struct rpc_task *bc_task;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; int err;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; struct xdr_buf *xbufp = &amp;req-&gt;rq_snd_buf;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; struct rpc_xprt *xprt = req-&gt;rq_xprt;<br>
<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * Create an rpc_task to send the data<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; bc_task = bc_alloc_task(req);<br>
<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * Reserve the xprt and then try to send the request across<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; err = bc_reserve_xprt(bc_task);<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (err)<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; goto out;<br>
<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; /*<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; * Set up the xdr_buf length<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; */<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; xbufp-&gt;len =
xbufp-&gt;head[0].iov_len + xbufp-&gt;page_len +
xbufp-&gt;tail[0].iov_len;<br>
<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; err = xprt-&gt;ops-&gt;send_request(bc_task);<br>
&nbsp;&nbsp;&nbsp; <br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; bc_release_xprt(bc_task);<br>
<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; xprt_free_bc_request(req);<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return 0;<br>
out:<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; bc_task-&gt;tk_rqstp = NULL;<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; bc_release_task(bc_task);<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return err;<br>
}<br>
<br>
Why do you return zero and not the err from xprt-&gt;ops-&gt;send_request?<br>
<br>
Here is what I suggest:<br>
<br>
<br>