[pnfs] [PATCH 18/37] pnfs: client layout cache: introduce get_alloc_layout

[Dean Hildebrand]

Benny Halevy wrote:
> get_alloc_layout gets or allocates if needed the pnfs_layout_type structure
> alloc_init_layout calls the layout driver's alloc_layout function
> and initializes the allocated pnfs_layout_type struct.
>
> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> ---
>  fs/nfs/pnfs.c             |   79 ++++++++++++++++++++++++++++++++-------------
>  include/linux/nfs4_pnfs.h |    1 +
>  include/linux/nfs_fs.h    |    2 +-
>  3 files changed, 58 insertions(+), 24 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index cb8115f..bd10670 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -403,38 +403,65 @@ out:
>   *     the I/O module has its read/write methods called.
>   */
>  static struct pnfs_layout_type *
> -pnfs_inject_layout(struct nfs_inode *nfsi,
> -		   struct layoutdriver_io_operations *io_ops,
> +pnfs_inject_layout(struct pnfs_layout_type *layid,
>  		   struct nfs4_pnfs_layoutget_res *lgr)
>  {
> -	struct pnfs_layout_type *layid;
> -	struct inode *inode = &nfsi->vfs_inode;
> -	struct nfs_server *server = NFS_SERVER(inode);
> +	struct layoutdriver_io_operations *io_ops = PNFS_LD_IO_OPS(layid);
>  
>  	dprintk("%s Begin\n", __FUNCTION__);
>  
> -	if (!io_ops->alloc_layout || !io_ops->set_layout) {
> +	if (!io_ops->set_layout) {
>  		printk(KERN_ERR "%s ERROR! Layout driver lacking pNFS layout ops!!!\n", __FUNCTION__);
>  		return NULL;
>  	}
>  
> -	if (nfsi->current_layout == NULL) {
> -		dprintk("%s Alloc'ing layout\n", __FUNCTION__);
> -		layid = io_ops->alloc_layout(server->pnfs_mountid, inode);
> -		if (layid)
> -			layid->inode = inode;
> -	} else {
> -		dprintk("%s Adding to current layout\n", __FUNCTION__);
> -		layid = nfsi->current_layout;
> -	}
> +	dprintk("%s Calling set layout\n", __FUNCTION__);
> +	return io_ops->set_layout(layid, lgr);
> +}
> +
> +static struct pnfs_layout_type *
> +alloc_init_layout(struct inode *ino, struct layoutdriver_io_operations *io_ops)
> +{
> +	struct pnfs_layout_type *lo;
>  
> -	if (!layid) {
> -		printk(KERN_ERR "%s ERROR! Layout id non-existent!!!\n",
> -		       __FUNCTION__);
> +	lo = io_ops->alloc_layout(NFS_SERVER(ino)->pnfs_mountid, ino);
>   
Since we checked for set_layout in pnfs_inject _layout, maybe we should 
check for alloc_layout here?
> +	if (!lo)
>  		return NULL;
> +
> +	lo->roc_iomode = 0;
> +	lo->inode = ino;
> +	return lo;
> +}
> +
> +/*
> + * get, possibly allocate current_layout
> + */
> +static struct pnfs_layout_type *
> +get_alloc_layout(struct inode *ino,
> +		   struct layoutdriver_io_operations *io_ops)
> +{
> +	struct nfs_inode *nfsi = NFS_I(ino);
> +	struct pnfs_layout_type *lo;
> +
> +	dprintk("%s Begin\n", __FUNCTION__);
> +
> +	lo = nfsi->current_layout;
> +	if (lo)
> +		goto out;
> +
> +	lo = nfsi->current_layout = alloc_init_layout(ino, io_ops);
> +	if (!lo) {
> +		lo = ERR_PTR(-ENOMEM);
> +		goto err;
>  	}
> -	dprintk("%s Calling set layout\n", __FUNCTION__);
> -	return io_ops->set_layout(layid, lgr);
> +
> +out:
> +	dprintk("%s Return %p\n", __FUNCTION__, lo);
> +	return lo;
> +
> +err:
> +	dprintk("%s Return error %ld\n", __FUNCTION__, PTR_ERR(lo));
> +	return lo;
>  }
>   
I like to use goto's if we can save duplicated code, but I'm thinking 
that these dprintk's could be moved to their respective places within 
the function since they are each called only once and actually increase 
the number of lines of code.  Also, if we are having memory problems, 
I'd be up for using a printk instead of a dprintk.
Dean
>  
>  /* Update the file's layout for the given range and iomode.
> @@ -454,6 +481,12 @@ pnfs_update_layout(struct inode *ino,
>  	struct pnfs_layout_type *layout_new;
>  	int result = -EIO;
>  
> +	layout_new = get_alloc_layout(ino, nfss->pnfs_curr_ld->ld_io_ops);
> +	if (IS_ERR(layout_new)) {
> +		result = PTR_ERR(layout_new);
> +		goto ret;
> +	}
> +
>  	arg.lseg.iomode = iomode;
>  	arg.lseg.offset = pos;
>  	arg.lseg.length = count;
> @@ -529,8 +562,7 @@ pnfs_update_layout(struct inode *ino,
>  	}
>  
>  	/* Inject layout blob into I/O device driver */
> -	layout_new = pnfs_inject_layout(nfsi,
> -					nfss->pnfs_curr_ld->ld_io_ops,
> +	layout_new = pnfs_inject_layout(nfsi->current_layout,
>  					&res);
>  	if (layout_new == NULL) {
>  		printk(KERN_ERR "%s: ERROR!  Could not inject layout (%d)\n",
> @@ -555,7 +587,8 @@ out:
>  
>  	/* res.layout.buf kalloc'ed by the xdr decoder? */
>  	kfree(res.layout.buf);
> -	dprintk("%s end (err:%d) state %d\n",
> +ret:
> +	dprintk("%s end (err:%d) state 0x%lx\n",
>  		__FUNCTION__, result, nfsi->pnfs_layout_state);
>  	return result;
>  }
> diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
> index d862aac..7ea5c33 100644
> --- a/include/linux/nfs4_pnfs.h
> +++ b/include/linux/nfs4_pnfs.h
> @@ -37,6 +37,7 @@ struct pnfs_mount_type {
>   * A reference is stored in the nfs_inode structure.
>   */
>  struct pnfs_layout_type {
> +	int refcount;
>  	int roc_iomode;			/* iomode to return on close, 0=none */
>  	struct inode *inode;
>  	u8 ld_data[];			/* layout driver private data */
> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
> index 548498d..3d6bfbf 100644
> --- a/include/linux/nfs_fs.h
> +++ b/include/linux/nfs_fs.h
> @@ -176,7 +176,7 @@ struct nfs_inode {
>  
>  	/* pNFS layout information */
>  #if defined(CONFIG_PNFS)
> -	u32 pnfs_layout_state;
> +	unsigned long pnfs_layout_state;
>  #define NFS_INO_LAYOUT_FAILED	0x0001	/* get layout failed, stop trying */
>  	time_t pnfs_layout_suspend;
>  	struct pnfs_layout_type *current_layout;
>