[pnfs] [PATCH 02/28] pnfs: fix pnfs_{read|write}pages

[Benny Halevy]

On Mar. 11, 2008, 21:31 +0200, Fred Isaman <iisaman at citi.umich.edu> wrote:
> THIS WILL BREAK OTHER LAYOUT DRIVERS that rely on old behavior.

Fred, do you have the cycles to fix the files and objects LDs
for the new behavior so we can merge this patch onto the pnfs branch?

> 
> This patch makes it possible to fall back to nfs from the layout driver's
> read_pagelist or write_pagelist routines. It alters the call chain, and
> expects ld_io_ops->{read|write}_pagelist to return either a 0 or 1,
> consistent with its parents, putting any other information that needs to
> be returned into the data.res and data.task structures.  The done routine
> should be called by the layout driver if and only if it returns 0.
> 
> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
> ---
>  fs/nfs/pnfs.c             |   22 ++++++++++++----------
>  include/linux/nfs4_pnfs.h |   16 ++++++++++++++--
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 8059181..7a7379d 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1422,13 +1422,15 @@ pnfs_writepages(struct nfs_write_data *wdata, int how)
>  							how,
>  							wdata);
>  
> -	if (status)
> +	if (status) {

a BUG_ON(status < 0) will be nice after all LDs are updated.

>  		put_lseg(lseg);
> -	if (status > 0) {
> -		dprintk("%s: LD write_pagelist returned status %d > 0\n", __FUNCTION__, status);
> -		pnfs_update_last_write(nfsi, args->offset, status);
> +		wdata->pnfsflags &= ~PNFS_NO_RPC;
> +	} else if (wdata->res.count > 0) {
> +		/* Succesful synchronous case */
> +		dprintk("%s: LD write_pagelist returned count %d > 0\n",
> +			__func__, wdata->res.count);
> +		pnfs_update_last_write(nfsi, args->offset, wdata->res.count);
>  		pnfs_need_layoutcommit(nfsi, wdata->args.context);

since the LD must call the done function also in the synchronous completion
case, pnfs_update_last_write and pnfs_need_layoutcommit are called there.

> -		status = 0;
>  	}
>  
>  out:
> @@ -1503,12 +1505,12 @@ pnfs_readpages(struct nfs_read_data *rdata)
>  							(loff_t)args->offset,
>  							args->count,
>  							rdata);
> -	if (status)
> +	if (status) {

ditto BUG_ON(status < 0);

>  		put_lseg(lseg);
> -	if (status > 0) {
> -		dprintk("%s: LD read_pagelist returned status %d > 0\n", __FUNCTION__, status);
> -		status = 0;
> -	}
> +		rdata->pnfsflags &= ~PNFS_NO_RPC;
> +	} else if (rdata->res.count > 0)
> +		dprintk("%s: LD read_pagelist returned count %d > 0\n",
> +			__func__, rdata->res.count);
>  
>   out:
>  	dprintk("%s: End Status %d\n", __FUNCTION__, status);
> diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
> index 381a599..2aaf526 100644
> --- a/include/linux/nfs4_pnfs.h
> +++ b/include/linux/nfs4_pnfs.h
> @@ -116,8 +116,20 @@ struct layoutdriver_io_operations {
>  	/* Functions that use the pagecache.
>  	 * If use_pagecache == 1, then these functions must be implemented.
>  	 */
> -	ssize_t (*read_pagelist) (struct pnfs_layout_type *layoutid, struct page **pages, unsigned int pgbase, unsigned nr_pages, loff_t offset, size_t count, struct nfs_read_data *nfs_data);
> -	ssize_t (*write_pagelist) (struct pnfs_layout_type *layoutid, struct page **pages, unsigned int pgbase, unsigned nr_pages, loff_t offset, size_t count, int sync, struct nfs_write_data *nfs_data);
> +	/* read and write pagelist should return just 0 (to indicate
> +	 * the layout code has taken control) or 1 (to indicate the
> +	 * layoutcode wishes to fall back to normal nfs.)  If 0 is returned,
> +	 * information can be passed back through nfs_data->res and
> +	 * nfs_data->task.tk_status.

"and the appropriate pnfs done function must be called (also when the I/O
completes synchronously.)"

Thanks,

Benny

> +	 */
> +	int (*read_pagelist) (struct pnfs_layout_type *layoutid,
> +			      struct page **pages, unsigned int pgbase,
> +			      unsigned nr_pages, loff_t offset, size_t count,
> +			      struct nfs_read_data *nfs_data);
> +	int (*write_pagelist) (struct pnfs_layout_type *layoutid,
> +			       struct page **pages, unsigned int pgbase,
> +			       unsigned nr_pages, loff_t offset, size_t count,
> +			       int sync, struct nfs_write_data *nfs_data);
>  	int (*flush_one) (struct pnfs_layout_segment *, struct list_head *head, unsigned int npages, size_t count, int how);
>  	void (*free_request_data) (struct nfs_page *);
>