[pnfs] [PATCH 13/14] pnfs: pnfs_modify_new_request
Dean Hildebrand
seattleplus at gmail.com
Wed Apr 2 14:33:39 EDT 2008
Benny Halevy wrote:
> On Apr. 02, 2008, 17:08 +0300, Benny Halevy <bhalevy at panasas.com> wrote:
>
>> 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.
>>
> To depict this model, I posted a chart here:
> http://www.bhalevy.com/pnfs/pnfs-page-sync-model.jpg
>
I like it, would you want to add lines for failure scenarios in the
diagram? (switching from the ld path to the mds path?)
Dean
>
> I also updated our issues wiki page:
> https://wiki.linux-nfs.org/wiki/index.php/PNFS_Implementation_Issues
>
> Benny
>
>
>> 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
>>>
>> _______________________________________________
>> pNFS mailing list
>> pNFS at linux-nfs.org
>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>>
>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
More information about the pNFS
mailing list