[pnfs] [PATCH 13/14] pnfs: pnfs_modify_new_request

[Benny Halevy]

Fredric Isaman wrote:
> 
> On Mon, 31 Mar 2008, Dean Hildebrand wrote:
> 
>>
>> Fred Isaman wrote:
>>> Add hook into nfs_page initialization to allow for setup of wb_private,
>>> and any other nfs_page fields that need twiddling.
>>>
>> It seems we have a general concept with fsdata (assume a boolean) storing 
>> whether or not the write request will use pnfs or not.  If so, we don't need 
>> to have a new interface op, 'new_request'.  Let's just have the general code 
>> check to see if fsdata == true (using pnfs) and then set PG_USE_PNFS in the 
>> nfs_page.wb_flags.  Essentially, I think your bl_new_request is general 
>> enough to move into pnfs_modify_new_request.
>>
>> Dean
>>
> 
> Note it is not the case that fsdata stores a boolean, it actually stores a 
> tri-state value, corresponding to:
> 1. READ
> 2. WRITE, so far ok to use pnfs
> 3. WRITE, must not use pnfs
> 
> I've no complaint about moving bl_new_request into 
> pnfs_modify_new_request, but I want to make sure everyone understands the 
> subtleties.  This would also require moving PG_USE_PNFS out of the 
> block-specific code, which again is reasonable to me, especially if others 
> can find use for it.

Eventually, I think that the concept of PG_USE_PNFS fits with the
overall page cache model I have in mind which will allow syncing via
either the MDS or the LD (and retrying and re-selecting if needed),
and then the page will continue through the respective commit path
which is, for MDS writes: sync NFS write or async + COMMIT, and
for LD writes: LD write, LD commit (if needed), and finally LAYOUTCOMMIT.

PG_USE_PNFS can be used, for example, on the retry path for forcing retry
through the MDS.

Benny

> 
>  	Fred
> 
>>> Signed-off-by: Fred Isaman <iisaman at citi.umich.edu>
>>> ---
>>>  fs/nfs/pagelist.c         |    1 +
>>>  fs/nfs/pnfs.c             |   15 +++++++++++++++
>>>  fs/nfs/pnfs.h             |   18 ++++++++++++++++++
>>>  include/linux/nfs4_pnfs.h |    4 ++++
>>>  4 files changed, 38 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
>>> index cfd3652..f365f7b 100644
>>> --- a/fs/nfs/pagelist.c
>>> +++ b/fs/nfs/pagelist.c
>>> @@ -88,6 +88,7 @@ nfs_create_request(struct nfs_open_context *ctx, struct 
>>> inode *inode,
>>>  	req->wb_bytes   = count;
>>>  	req->wb_context = get_nfs_open_context(ctx);
>>>  	kref_init(&req->wb_kref);
>>> +	pnfs_modify_new_request(inode, req, fsdata);
>>>  	return req;
>>>  }
>>>  diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>> index 9025132..2774a6d 100644
>>> --- a/fs/nfs/pnfs.c
>>> +++ b/fs/nfs/pnfs.c
>>> @@ -1932,6 +1932,21 @@ out_unlock:
>>>  	goto out;
>>>  }
>>>  +void _pnfs_modify_new_request(struct nfs_server *nfss, struct inode 
>>> *inode,
>>> +			      struct nfs_page *req, struct pnfs_fsdata 
>>> *fsdata)
>>> +{
>>> +	struct pnfs_layout_segment *lseg = NULL;
>>> +	loff_t pos;
>>> +	unsigned count;
>>> +
>>> +	pos = ((loff_t)req->wb_index << PAGE_CACHE_SHIFT) + req->wb_offset;
>>> +	count = req->wb_bytes;
>>> +	lseg = pnfs_find_get_lseg(inode, pos, count, IOMODE_RW);
>>> +	nfss->pnfs_curr_ld->ld_io_ops->new_request(lseg, req, pos, count,
>>> +						   fsdata);
>>> +	put_lseg(lseg);
>>> +}
>>> +
>>>  void pnfs_free_request_data(struct nfs_page *req)
>>>  {
>>>  	struct layoutdriver_io_operations *lo;
>>> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
>>> index 44d9f35..5948be4 100644
>>> --- a/fs/nfs/pnfs.h
>>> +++ b/fs/nfs/pnfs.h
>>> @@ -58,6 +58,8 @@ int _pnfs_write_begin(struct inode *inode, struct 
>>> nfs_server *nfss,
>>>  		      unsigned flags, struct pnfs_fsdata **fsdata);
>>>  int _pnfs_do_flush(struct inode *inode, struct nfs_server *nfss,
>>>  		   struct nfs_page *req, struct pnfs_fsdata *fsdata);
>>> +void _pnfs_modify_new_request(struct nfs_server *nfss, struct inode 
>>> *inode,
>>> +			      struct nfs_page *req, struct pnfs_fsdata 
>>> *fsdata);
>>>   #define PNFS_EXISTS_LDIO_OP(opname) (nfss->pnfs_curr_ld && \
>>>  				     nfss->pnfs_curr_ld->ld_io_ops && \
>>> @@ -96,6 +98,16 @@ static inline void pnfs_write_end_cleanup(void *fsdata)
>>>  {
>>>  	pnfs_free_fsdata(fsdata);
>>>  }
>>> +
>>> +static inline void pnfs_modify_new_request(struct inode *inode,
>>> +					   struct nfs_page *req,
>>> +					   void *fsdata)
>>> +{
>>> +	struct nfs_server *nfss = NFS_SERVER(inode);
>>> +	if (PNFS_EXISTS_LDIO_OP(new_request))
>>> +		_pnfs_modify_new_request(nfss, inode, req, fsdata);
>>> +}
>>> +
>>>  #else /* CONFIG_PNFS */
>>>   static inline int pnfs_do_flush(struct nfs_page *req, void *fsdata)
>>> @@ -114,6 +126,12 @@ static inline void pnfs_write_end_cleanup(void 
>>> *fsdata)
>>>  {
>>>  }
>>>  +static inline void pnfs_modify_new_request(struct inode *inode,
>>> +					   struct nfs_page *req,
>>> +					   void *fsdata)
>>> +{
>>> +}
>>> +
>>>  #endif /* CONFIG_PNFS */
>>>   #endif /* FS_NFS_PNFS_H */
>>> diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
>>> index f711df2..ea1d308 100644
>>> --- a/include/linux/nfs4_pnfs.h
>>> +++ b/include/linux/nfs4_pnfs.h
>>> @@ -142,6 +142,10 @@ struct layoutdriver_io_operations {
>>>  	int (*write_begin) (struct pnfs_layout_segment *lseg, struct page 
>>> *page,
>>>  			    loff_t pos, unsigned count,
>>>  			    struct pnfs_fsdata **fsdata);
>>> +	/* Hook into nfs_create_request, for setting wb_private */
>>> +	void (*new_request)(struct pnfs_layout_segment *lseg,
>>> +			    struct nfs_page *req, loff_t pos, unsigned count,
>>> +			    struct pnfs_fsdata *fsdata);
>>>  	void (*free_request_data) (struct nfs_page *);
>>>  	void (*free_fsdata) (struct pnfs_fsdata *data);
>>>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs