[pnfs] [PATCH 4/8] pnfs: split pnfs_update_layout into done routine
Benny Halevy
bhalevy at panasas.com
Wed Mar 12 10:19:06 EDT 2008
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,
> };
> 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;
originally, and down below in pnfs_get_layout_done the
assignment to *lsegpp is done also when lseg==NULL.
Not a big deal but let's keep it consistent.
> + 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);
lgp->status is not being set in this path, is it?
> 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;
why don't we do that right before the switch. It is overridden anyway
where we explicitly set it...
> 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);
nit: extra space after the '='
> 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;
that should be done inside the lo_lock as |= isn't atomic
Benny
>
> - /* 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 {
More information about the pNFS
mailing list