[pnfs] [PATCH 19/28] pnfs: pnfs_write_begin

[Fredric Isaman]

On Thu, 13 Mar 2008, Benny Halevy wrote:

> On Mar. 11, 2008, 21:32 +0200, Fred Isaman <iisaman at citi.umich.edu> wrote:
>> From: Fred <iisaman at citi.umich.edu>
>>
>> Add hooks in the nfs_write_begin path, giving a driver the potential
>> to read in page data around the data that is about to be copied to the page.
>>
>> Signed-off-by: Fred <iisaman at citi.umich.edu>
>> ---
>>  fs/nfs/file.c             |   20 ++++++++--
>>  fs/nfs/pnfs.c             |   88 +++++++++++++++++++++++++++++++++++++++++++++
>>  fs/nfs/pnfs.h             |   10 +++++
>>  fs/nfs/write.c            |    2 +-
>>  include/linux/nfs4_pnfs.h |   15 +++++++-
>>  5 files changed, 129 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 6f16772..e870982 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -352,10 +352,19 @@ static int nfs_write_begin(struct file *file, struct address_space *mapping,
>>  	*pagep = page;
>>
>>  	ret = nfs_flush_incompatible(file, page);
>> -	if (ret) {
>> -		unlock_page(page);
>> -		page_cache_release(page);
>> -	}
>> +	if (ret)
>> +		goto out_err;
>> +#ifdef CONFIG_PNFS
>> +	ret = pnfs_write_begin(file, page, pos, len, flags, fsdata);
>> +	if (ret)
>> +		goto out_err;
>> +#endif /* CONFIG_PNFS */
>> +	return 0;
>> +
>> + out_err:
>> +	unlock_page(page);
>> +	page_cache_release(page);
>> +	*pagep = NULL;
>>  	return ret;
>>  }
>>
>> @@ -372,6 +381,9 @@ static int nfs_write_end(struct file *file, struct address_space *mapping,
>>
>>  	unlock_page(page);
>>  	page_cache_release(page);
>> +#ifdef CONFIG_PNFS
>> +	pnfs_free_fsdata(fsdata);
>> +#endif /* CONFIG_PNFS */
>>
>>  	return status < 0 ? status : copied;
>>  }
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index 56aa1bf..a4db86e 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -809,6 +809,28 @@ pnfs_has_layout(struct pnfs_layout_type *lo,
>>  	return ret;
>>  }
>>
>> +static struct pnfs_layout_segment *
>> +pnfs_find_get_lseg(struct inode *inode,
>> +		   loff_t pos,
>> +		   size_t count,
>> +		   enum pnfs_iomode iomode)
>> +{
>> +	struct nfs_inode *nfsi = NFS_I(inode);
>> +	struct pnfs_layout_segment *lseg;
>> +	struct pnfs_layout_type *lo;
>> +	struct nfs4_pnfs_layout_segment range;
>> +
>> +	lo = get_lock_current_layout(nfsi);
>> +	if (!lo)
>> +		return NULL;
>> +	range.iomode = iomode;
>> +	range.offset = pos;
>> +	range.length = count;
>> +	lseg = pnfs_has_layout(lo, &range, 1);
>> +	put_unlock_current_layout(nfsi, lo);
>> +	return lseg;
>> +}
>> +
>>  /* Update the file's layout for the given range and iomode.
>>   * Layout is retreived from the server if needed.
>>   * If lsegpp is given, the appropriate layout segment is referenced and
>> @@ -1537,6 +1559,64 @@ int pnfs_try_to_read_data(struct nfs_read_data *data,
>>  	}
>>  }
>>
>> +/*
>> + * This gives the layout driver an opportunity to read in page "around"
>> + * the data to be written.  It returns 0 on success, otherwise an error code
>> + * which will either be passed up to user, or ignored if
>> + * some previous part of write succeeded.
>> + * Note the range [pos, pos+len-1] is entirely within the page.
>> + */
>> +/* flags = AOP_FLAG_UNINTERRUPTIBLE or 0 - ??? need this ??? */
>> +int pnfs_write_begin(struct file *filp, struct page *page,
>> +		     loff_t pos, unsigned len,
>> +		     unsigned flags, void **fsdata)
>> +{
>> +	struct inode *inode = filp->f_dentry->d_inode;
>> +	struct nfs_server *nfss = NFS_SERVER(inode);
>> +	struct pnfs_layout_segment *lseg;
>> +	struct pnfs_fsdata *pnfs_fsdata = NULL;
>> +	int status = 0;
>> +
>> +	if (!nfss->pnfs_curr_ld || !nfss->pnfs_curr_ld->ld_io_ops ||
>> +	    !nfss->pnfs_curr_ld->ld_io_ops->write_begin) {
>> +		/* If layout driver doesn't define, just do nothing */
>> +		goto out;
>> +	}
>> +	lseg = pnfs_find_get_lseg(inode, pos, len, IOMODE_RW);
>> +	status = nfss->pnfs_curr_ld->ld_io_ops->write_begin(lseg, page,
>> +							    pos, len,
>> +							    &pnfs_fsdata);
>> +	put_lseg(lseg);
>> +	if (pnfs_fsdata && !pnfs_fsdata->ld_io_ops)
>> +		pnfs_fsdata->ld_io_ops = nfss->pnfs_curr_ld->ld_io_ops;
>> + out:
>> +	*fsdata = pnfs_fsdata;
>
> One more comment. Can fsdata be NULL?
>
> Benny
>
>

Good question.  Not in the path through generic_perform_write.  I'll look 
for other paths, and fix the function if needed.

 	Fred