[pnfs] [PATCH 7/8] pnfs: refactor pnfs_return_layout
Benny Halevy
bhalevy at panasas.com
Wed Mar 12 11:16:41 EDT 2008
On Mar. 10, 2008, 20:39 +0200, andros at umich.edu wrote:
> From: Andy Adamson <andros at umich.edu>
>
> Remove pnfs_return_layout_rpc(). Add a recall type parameter to
> pnfs_return_layout() and simplify calls in pnfs_recall_layout().
>
> Signed-off-by: Andy Adamson<andros at umich.edu>
> ---
> fs/nfs/callback_proc.c | 19 +++++-----------
> fs/nfs/inode.c | 2 +-
> fs/nfs/nfs4proc.c | 2 +-
> fs/nfs/nfs4state.c | 2 +-
> fs/nfs/pnfs.c | 53 ++++++++++++-----------------------------------
> fs/nfs/pnfs.h | 4 +-
> 6 files changed, 25 insertions(+), 57 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 18fa030..b4bbb3d 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -148,11 +148,9 @@ static int pnfs_recall_layout(void *data)
> {
> struct inode *inode, *ino;
> struct nfs_client *clp;
> - struct nfs_server *server = NULL;
> struct cb_pnfs_layoutrecallargs rl;
> struct recall_layout_threadargs *args =
> (struct recall_layout_threadargs *)data;
> - struct nfs4_pnfs_layoutreturn_arg lr_arg;
> int status;
>
> daemonize("nfsv4-layoutreturn");
> @@ -163,7 +161,6 @@ static int pnfs_recall_layout(void *data)
>
> clp = args->clp;
> inode = args->inode;
> - server = NFS_SERVER(inode);
> rl = args->rl;
> args->result = 0;
> complete(&args->started);
> @@ -180,7 +177,8 @@ static int pnfs_recall_layout(void *data)
> */
>
> if (rl.cbl_recall_type == RECALL_FILE) {
> - pnfs_return_layout(inode, &rl.cbl_seg);
> + /* XXX need to check status on pnfs_return_layout */
we may want to print a debug message on error, but the status
is not percolating up as we're running in a kthread context.
> + pnfs_return_layout(inode, &rl.cbl_seg, RECALL_FILE);
> goto out;
> }
>
> @@ -189,21 +187,16 @@ static int pnfs_recall_layout(void *data)
>
> /* FIXME: This loop is inefficient, running in O(|s_inodes|^2) */
> while ((ino = nfs_layoutrecall_find_inode(clp, &rl)) != NULL) {
> - pnfs_return_layout(ino, &rl.cbl_seg);
> + /* XXX need to check status on pnfs_return_layout */
> + pnfs_return_layout(ino, &rl.cbl_seg, RECALL_FILE);
> iput(ino);
> }
>
> /* send final layoutreturn */
> - lr_arg.reclaim = 0;
> - lr_arg.layout_type = server->pnfs_curr_ld->id;
> - lr_arg.return_type = rl.cbl_recall_type;
> - lr_arg.lseg = rl.cbl_seg;
> - lr_arg.inode = inode;
> -
> - status = pnfs_return_layout_rpc(server, &lr_arg);
> + status = pnfs_return_layout(inode, &rl.cbl_seg, rl.cbl_recall_type);
> if (status)
> printk(KERN_INFO
> - "%s: ignoring pnfs_return_layout_rpc status=%d\n",
> + "%s: ignoring pnfs_return_layout status=%d\n",
> __func__, status);
> out:
> iput(inode);
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index b0c5308..38c9359 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1151,7 +1151,7 @@ void nfs4_clear_inode(struct inode *inode)
> /* First call standard NFS clear_inode() code */
> nfs_clear_inode(inode);
> #ifdef CONFIG_PNFS
> - pnfs_return_layout(inode, NULL);
> + pnfs_return_layout(inode, NULL, RECALL_FILE);
> #endif /* CONFIG_PNFS */
> }
> #endif
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 3634814..729e14e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2160,7 +2160,7 @@ pnfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
> if (p_ops && p_ops->layoutret_on_setattr) {
> if (nfsi->layoutcommit_ctx)
> pnfs_layoutcommit_inode(inode, 0);
> - pnfs_return_layout(inode, NULL);
> + pnfs_return_layout(inode, NULL, RECALL_FILE);
> }
> }
> return nfs4_proc_setattr(dentry, fattr, sattr);
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index db7222f..3a4e16e 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -508,7 +508,7 @@ static void __nfs4_close(struct path *path, struct nfs4_state *state, mode_t mod
> range.iomode = nfsi->current_layout->roc_iomode;
> range.offset = 0;
> range.length = NFS4_LENGTH_EOF;
> - pnfs_return_layout(state->inode, &range);
> + pnfs_return_layout(state->inode, &range, RECALL_FILE);
> }
> #endif /* CONFIG_PNFS */
> nfs4_do_close(path, state, wait);
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index d33fcc7..e4ed81f 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -535,23 +535,29 @@ pnfs_free_layout(struct pnfs_layout_type *lo,
> }
>
> int
> -pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range)
> +pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
> + enum pnfs_layoutreturn_type type)
> {
> + struct nfs4_pnfs_layoutreturn_arg arg;
> + struct nfs4_pnfs_layoutreturn_res res;
> + struct nfs4_pnfs_layoutreturn gdata = {
> + .args = &arg,
> + .res = &res,
> + };
> struct pnfs_layout_type *lo;
> struct nfs_inode *nfsi = NFS_I(ino);
> struct nfs_server *server = NFS_SERVER(ino);
> - struct nfs4_pnfs_layoutreturn_arg arg;
> int status;
>
> lo = get_lock_current_layout(nfsi);
needed only for type == RETURN_FILE
(I suggest moving it down below the arg setup)
> - dprintk("%s:Begin layout %p\n", __FUNCTION__, lo);
> -
> if (lo == NULL)
> return 0;
lo may be NULL when doing the final return for RETURN_{FSID,ALL}
>
> + dprintk("%s:Begin layout %p\n", __FUNCTION__, lo);
s/FUNCTION/func/ please
> +
> arg.reclaim = 0;
> arg.layout_type = server->pnfs_curr_ld->id;
> - arg.return_type = RETURN_FILE;
> + arg.return_type = type;
> if (range)
> arg.lseg = *range;
> else {
> @@ -562,44 +568,13 @@ pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range)
> arg.inode = ino;
>
> pnfs_free_layout(lo, &arg.lseg);
This mustn't be called when type != RETURN_FILE.
> - put_unlock_current_layout(nfsi, lo);
needed only for type == RETURN_FILE
in summary this block can be done after setting up arg, like this:
if (type == RETURN_FILE) {
lo = get_lock_current_layout(nfsi);
if (lo) {
pnfs_free_layout(lo, &arg.lseg);
put_unlock_current_layout(nfsi, lo);
}
}
> -
> - status = pnfs_return_layout_rpc(server, &arg);
> -
> - dprintk("%s:Exit status %d\n", __FUNCTION__, status);
> - return status;
> -}
> -
> -int
> -pnfs_return_layout_rpc(struct nfs_server *server,
> - struct nfs4_pnfs_layoutreturn_arg *argp)
> -{
> - int status;
> - struct nfs4_pnfs_layoutreturn_res res;
> - struct nfs4_pnfs_layoutreturn gdata = {
> - .args = argp,
> - .res = &res,
> - };
> - dprintk("%s:Begin\n", __FUNCTION__);
>
> - /* XXX Need to setup the sequence */
> -/*
> - status = server->nfs_client->rpc_ops->setup_sequence(
> - server->session,
> - argp->minorversion_info,
> - res.minorversion_info)
> - if (status)
> - goto out;
> -*/
> - /* Return layout to server */
> + spin_unlock(&nfsi->lo_lock);
only for type == RETURN_FILE.
(there was a reason why I originally split the return function from
the return_rpc :)
> status = server->nfs_client->rpc_ops->pnfs_layoutreturn(&gdata);
> + spin_lock(&nfsi->lo_lock);
ditto
>
> -/*
> - server->nfs_client->rpc_ops->sequence_done(server->session,
> - res.minorversion_info, status);
> + put_unlock_current_layout(nfsi, lo);
ditto
hmm, actually rather than doing that:
unlock()
do the rpc
lock()
put_unlock()
the example I gave above already put_unlock's the layout at
the right time and just for RETURN_FILE so there's no need
to joggle the lock anymore around the rpc call.
Benny
>
> -out:
> -*/
> dprintk("%s:Exit status %d\n", __FUNCTION__, status);
> return status;
> }
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index db25489..0e0e678 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -20,8 +20,8 @@ int pnfs_update_layout(struct inode *ino, struct nfs_open_context *ctx,
> size_t count, loff_t pos, enum pnfs_iomode access_type,
> struct pnfs_layout_segment **lsegpp);
>
> -int pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range);
> -int pnfs_return_layout_rpc(struct nfs_server *server, struct nfs4_pnfs_layoutreturn_arg *argp);
> +int pnfs_return_layout(struct inode *, struct nfs4_pnfs_layout_segment *,
> + enum pnfs_layoutreturn_type);
> void set_pnfs_layoutdriver(struct super_block *sb, struct nfs_fh *fh, u32 id);
> void unmount_pnfs_layoutdriver(struct super_block *sb);
> int pnfs_use_read(struct inode *inode, ssize_t count);
More information about the pNFS
mailing list