[pnfs] [PATCH 4/8] pnfs: split pnfs_update_layout into done routine

William A. (Andy) Adamson andros at citi.umich.edu
Wed Mar 12 11:01:51 EDT 2008


On Wed, Mar 12, 2008 at 10:43 AM, Benny Halevy <bhalevy at panasas.com> wrote:

> On Mar. 10, 2008, 20:39 +0200, andros at umich.edu wrote:
> > From: Andy Adamson <andros at umich.edu>
> >
> > The new async nfs4_pnfs_layoutget_done() calls the bottom half of
> > pnfs_update_layout() now called pnfs_update_layout_done().
> > Replace the call to put_unlock_current_layout(), which will go into the
> rpc
> > release call, with spin_unlock().
> >
> > Signed-off-by: Andy Adamson<andros at umich.edu>
> > ---
> >  fs/nfs/nfs4proc.c        |    9 ++++-
> >  fs/nfs/pnfs.c            |   99
> +++++++++++++++++++++++++++++-----------------
> >  fs/nfs/pnfs.h            |    3 +
> >  include/linux/pnfs_xdr.h |    2 +
> >  4 files changed, 76 insertions(+), 37 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index bcf9ea6..ef77a89 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5253,12 +5253,19 @@ static void nfs4_pnfs_layoutget_done(struct
> rpc_task *task, void *calldata)
> >       struct nfs4_pnfs_layoutget *lgp = calldata;
> >       struct inode *ino = lgp->args->inode;
> >       struct nfs_server *server = NFS_SERVER(ino);
> > +     struct nfs_inode *nfsi = NFS_I(ino);
> > +     struct pnfs_layout_type *lo;
> >
> >       dprintk("--> %s\n", __func__);
> >       if (RPC_ASSASSINATED(task))
> >               return;
> >
> >       nfs4_sequence_done(server, &lgp->res->seq_res, task->tk_status);
> > +
> > +     lo = nfsi->current_layout;
> > +     BUG_ON(!lo);
> > +
> > +     pnfs_get_layout_done(lo, lgp, task->tk_status);
> >       dprintk("<-- %s\n", __func__);
> >  }
> >
> > @@ -5294,7 +5301,7 @@ static int nfs4_proc_pnfs_layoutget(struct
> nfs4_pnfs_layoutget *lgp)
> >       if (status != 0) {
> >               smp_wmb();
> >       } else
> > -             status = task->tk_status;
> > +             status = lgp->status;
> >       rpc_put_task(task);
> >
> >       dprintk("<-- %s\n", __func__);
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index 31158b6..7665188 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -825,6 +825,7 @@ pnfs_update_layout(struct inode *ino,
> >       struct nfs4_pnfs_layoutget gdata = {
> >               .args = &arg,
> >               .res = &res,
> > +             .lsegpp = lsegpp,
> >       };
>
> Oh, gdata, arg, and res can't be allocated on the stack
> and passed to the async rpc!  You must dynamically allocated them
> and free in the _release rpc method.


ulp - same with layoutget.


>
> It's going to be easier if the args and res will be
> embedded in struct nfs4_pnfs_layoutget
>
> How about renaming it and redefining as follows
> (so it will be consistent with present rpc callback data structs):
>
> struct pnfs_layoutget_data {
>         struct nfs4_pnfs_layoutget_arg args;
>        struct nfs4_pnfs_layoutget_res res;
>        struct pnfs_layout_segment **lsegpp;
>         int rpc_status;
> };
>
> See nfs4_opendata for example.
>
> Benny
>
> >       struct nfs_inode *nfsi = NFS_I(ino);
> >       struct nfs_server *nfss = NFS_SERVER(ino);
> > @@ -834,8 +835,9 @@ pnfs_update_layout(struct inode *ino,
> >
> >       lo = get_lock_alloc_layout(ino, nfss->pnfs_curr_ld->ld_io_ops);
> >       if (IS_ERR(lo)) {
> > +             dprintk("%s ERROR: can't get pnfs_layout_type\n",
> __func__);
> >               result = PTR_ERR(lo);
> > -             goto ret;
> > +             goto out;
> >       }
> >
> >       arg.lseg.iomode = iomode;
> > @@ -845,110 +847,135 @@ pnfs_update_layout(struct inode *ino,
> >       lseg = pnfs_has_layout(lo, &arg.lseg, lsegpp != NULL);
> >       if (lseg) {
> >               dprintk("%s: Using cached layout %p for %llu@%llu iomode
> %d)\n",
> > -                     __FUNCTION__,
> > +                     __func__,
> >                       nfsi->current_layout,
> >                       arg.lseg.length,
> >                       arg.lseg.offset,
> >                       arg.lseg.iomode);
> >
> >               result = 0;
> > -             goto out;
> > +             goto out_put;
> >       }
> >
> >       /* if get layout already failed once goto out */
> >       if (nfsi->pnfs_layout_state & NFS_INO_LAYOUT_FAILED) {
> >               if (unlikely(nfsi->pnfs_layout_suspend &&
> >                   get_seconds() >= nfsi->pnfs_layout_suspend)) {
> > -                     dprintk("%s: layout_get resumed\n", __FUNCTION__);
> > +                     dprintk("%s: layout_get resumed\n", __func__);
> >                       nfsi->pnfs_layout_state &= ~NFS_INO_LAYOUT_FAILED;
> >                       nfsi->pnfs_layout_suspend = 0;
> >               } else {
> >                       result = 1;
> > -                     goto out;
> > +                     goto out_put;
> >               }
> >       }
> >
> >       res.layout.buf = NULL;
> >       memcpy(&lo->stateid.data, &arg.stateid.data, NFS4_STATEID_SIZE);
> >       spin_unlock(&nfsi->lo_lock);
> > +
> >       result = get_layout(ino, ctx, &gdata);
> > +out:
> > +     dprintk("%s end (err:%d) state 0x%lx lseg %p\n",
> > +                     __func__, result, nfsi->pnfs_layout_state, lseg);
> > +     return result;
> > +out_put:
> > +     if (lseg && lsegpp)
> > +             *lsegpp = lseg;
> > +     put_unlock_current_layout(nfsi, lo);
> > +     goto out;
> > +}
> > +
> > +void
> > +pnfs_get_layout_done(struct pnfs_layout_type *lo,
> > +                  struct nfs4_pnfs_layoutget *lgp,
> > +                  int rpc_status)
> > +{
> > +     struct nfs4_pnfs_layoutget_res *res = lgp->res;
> > +     struct pnfs_layout_segment *lseg = NULL;
> > +     struct nfs_inode *nfsi = NFS_I(lo->inode);
> > +
> > +     dprintk("-->%s \n", __func__);
> > +
> >       spin_lock(&nfsi->lo_lock);
> > -     /* FIXME: check for reordering using the returned sequence id */
> > -     memcpy(&res.stateid.data, &lo->stateid.data, NFS4_STATEID_SIZE);
> >
> > -     /* we got a reference on nfsi->current_layout hence it must never
> > -      * change, even while nfsi->lo_lock was not held.
> > -      */
> >       BUG_ON(nfsi->current_layout != lo);
> >
> > -     if (result) {
> > +     /* FIXME: check for reordering using the returned sequence id */
> > +     memcpy(&res->stateid.data, &lo->stateid.data, NFS4_STATEID_SIZE);
> > +
> > +     if (rpc_status) {
> >               dprintk("%s: ERROR retrieving layout %d\n",
> > -                    __FUNCTION__, result);
> > +                    __func__, rpc_status);
> >
> > -             switch (result) {
> > +             switch (rpc_status) {
> >               case -ENOENT:   /* NFS4ERR_BADLAYOUT */
> >                       /* transient error, don't mark with
> NFS_INO_LAYOUT_FAILED */
> > -                     result = 1;
> > +                     lgp->status = 1;
> >                       break;
> >
> > -             case -EAGAIN:   /* NFS4ERR_LAYOUTTRYLATER,
> NFS4ERR_RECALLCONFLICT, NFS4ERR_LOCKED */
> > +             case -EAGAIN:   /* NFS4ERR_LAYOUTTRYLATER,
> > +                              * NFS4ERR_RECALLCONFLICT, NFS4ERR_LOCKED
> > +                              */
> >                       nfsi->pnfs_layout_suspend = get_seconds() + 1;
> >                       dprintk("%s: layout_get suspended until %ld\n",
> > -                             __FUNCTION__, nfsi->pnfs_layout_suspend);
> > +                             __func__, nfsi->pnfs_layout_suspend);
> >                       break;
> > -             case -EINVAL:   /* NFS4ERR_INVAL, NFSERR_BADIOMODE,
> NFS4ERR_UNKNOWN_LAYOUTTYPE */
> > +             case -EINVAL:   /* NFS4ERR_INVAL, NFSERR_BADIOMODE,
> > +                              * NFS4ERR_UNKNOWN_LAYOUTTYPE
> > +                              */
> >               case -ENOTSUPP: /* NFS4ERR_LAYOUTUNAVAILABLE */
> >               case -ETOOSMALL:/* NFS4ERR_TOOSMALL */
> >               default:
> >                       /* suspend layout get for ever for this file */
> >                       nfsi->pnfs_layout_suspend = 0;
> >                       dprintk("%s: no layout_get until %ld\n",
> > -                             __FUNCTION__, nfsi->pnfs_layout_suspend);
> > +                             __func__, nfsi->pnfs_layout_suspend);
> >                       /* mark with NFS_INO_LAYOUT_FAILED */
> > +                     lgp->status = rpc_status;
> >                       break;
> >               }
> >               goto get_out;
> >       }
> >
> > -     if (res.layout.len <= 0) {
> > +     if (res->layout.len <= 0) {
> >               printk(KERN_ERR
> >                      "%s: ERROR!  Layout size is ZERO!\n",
> __FUNCTION__);
> > -             result =  -EIO;
> > +             lgp->status =  -EIO;
> >               goto get_out;
> >       }
> >
> >       /* Inject layout blob into I/O device driver */
> > -     lseg = pnfs_inject_layout(lo, &res, lsegpp != NULL);
> > +     lseg = pnfs_inject_layout(lo, res, lgp->lsegpp != NULL);
> >       if (IS_ERR(lseg)) {
> > -             result =  PTR_ERR(lseg);
> > +             lgp->status =  PTR_ERR(lseg);
> >               lseg = NULL;
> >               printk(KERN_ERR "%s: ERROR!  Could not inject layout
> (%d)\n",
> > -                    __FUNCTION__, result);
> > +                    __func__, lgp->status);
> >               goto get_out;
> >       }
> >
> > -     if (res.return_on_close) {
> > -             lo->roc_iomode |= res.lseg.iomode;
> > +     if (res->return_on_close) {
> > +             lo->roc_iomode |= res->lseg.iomode;
> >               if (!lo->roc_iomode)
> >                       lo->roc_iomode = IOMODE_ANY;
> >       }
> > +     lgp->status = 0;
> >
> > -     result = 0;
> >  get_out:
> > +     spin_unlock(&nfsi->lo_lock);
> >       /* remember that get layout failed and don't try again */
> > -     if (result < 0)
> > +     if (lgp->status < 0)
> >               nfsi->pnfs_layout_state |= NFS_INO_LAYOUT_FAILED;
> >
> > -     /* res.layout.buf kalloc'ed by the xdr decoder? */
> > -     kfree(res.layout.buf);
> > -out:
> > -     put_unlock_current_layout(nfsi, lo);
> > -ret:
> > +     /* res->layout.buf kalloc'ed by the xdr decoder? */
> > +     kfree(res->layout.buf);
> > +
> >       dprintk("%s end (err:%d) state 0x%lx lseg %p\n",
> > -             __FUNCTION__, result, nfsi->pnfs_layout_state, lseg);
> > -     if (lsegpp)
> > -             *lsegpp = lseg;
> > -     return result;
> > +                     __func__, lgp->status, nfsi->pnfs_layout_state,
> lseg);
> > +     if (lgp->lsegpp)
> > +             *lgp->lsegpp = lseg;
> > +     return;
> >  }
> >
> >  /* Return true if a layout driver is being used for this mountpoint */
> > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> > index 8e11489..c458b18 100644
> > --- a/fs/nfs/pnfs.h
> > +++ b/fs/nfs/pnfs.h
> > @@ -47,6 +47,9 @@ void pnfs_update_layout_commit(struct inode *, struct
> list_head *, pgoff_t, unsi
> >  int pnfs_flush_one(struct inode *, struct list_head *, unsigned int,
> size_t, int);
> >  void pnfs_free_request_data(struct nfs_page *req);
> >  ssize_t pnfs_file_write(struct file *, const char __user *, size_t,
> loff_t *);
> > +void pnfs_get_layout_done(struct pnfs_layout_type *,
> > +                       struct nfs4_pnfs_layoutget *, int);
> > +
> >
> >  #endif /* CONFIG_PNFS */
> >
> > diff --git a/include/linux/pnfs_xdr.h b/include/linux/pnfs_xdr.h
> > index c6a4e3a..039c271 100644
> > --- a/include/linux/pnfs_xdr.h
> > +++ b/include/linux/pnfs_xdr.h
> > @@ -56,6 +56,8 @@ struct nfs4_pnfs_layoutget_res {
> >  struct nfs4_pnfs_layoutget {
> >       struct nfs4_pnfs_layoutget_arg *args;
> >       struct nfs4_pnfs_layoutget_res *res;
> > +     struct pnfs_layout_segment **lsegpp;
> > +     int status;
> >  };
> >
> >  struct pnfs_layoutcommit_arg {
>
> _______________________________________________
> 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/20080312/f8c7264a/attachment-0001.htm 


More information about the pNFS mailing list