[pnfs] [PATCH 2/2] bc_send: handle error from bc_alloc_task

[William A. (Andy) Adamson]

On 5/25/07, William A. (Andy) Adamson <andros at citi.umich.edu> wrote:
>
> This patch did not apply to the 4.1-sessions branch. There is no
> xprt_free_bc_request(req); after the out: label.


ok!  i just looked at the first patch in the series which adds the
xprt_free_bc_request(req) aftet the out: label. still need to return the
error from send_request....

-->Andy

here is the function in the 4.1-sessions branch.
>
> /*
>  * Send the callback reply
>  */
> int bc_send(struct rpc_rqst *req)
> {
>         struct rpc_task *bc_task;
>         int err;
>         struct xdr_buf *xbufp = &req->rq_snd_buf;
>         struct rpc_xprt *xprt = req->rq_xprt;
>
>         /*
>          * Create an rpc_task to send the data
>          */
>         bc_task = bc_alloc_task(req);
>
>         /*
>          * Reserve the xprt and then try to send the request across
>          */
>         err = bc_reserve_xprt(bc_task);
>         if (err)
>                 goto out;
>
>         /*
>          * Set up the xdr_buf length
>          */
>         xbufp->len = xbufp->head[0].iov_len + xbufp->page_len +
> xbufp->tail[0].iov_len;
>
>         err = xprt->ops->send_request(bc_task);
>
>         bc_release_xprt(bc_task);
>
>         xprt_free_bc_request(req);
>         return 0;
> out:
>         bc_task->tk_rqstp = NULL;
>         bc_release_task(bc_task);
>         return err;
> }
>
> Why do you return zero and not the err from xprt->ops->send_request?
>
> Here is what I suggest:
>
>
> From 3db8f71d776d026ce8b07621a9947ee432ab0b05 Mon Sep 17 00:00:00 2001
> From: Andy Adamson <andros at umich.edu>
> Date: Fri, 25 May 2007 12:38:41 -0400
> Subject: [PATCH 3/3] Return errors in bc_send
>
> 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070525/c198bfe5/attachment-0001.htm