On 2011-06-06 18:32, Trond Myklebust wrote:
> We need to ensure that the layouts are set up before we can decide to
> coalesce requests. To do so, we want to further split up the struct
> nfs_pageio_descriptor operations into an initialisation callback, a
> coalescing test callback, and a 'do i/o' callback.
>
> This patch cleans up the existing callback methods before adding the
> 'initialisation' callback.
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> ---
> fs/nfs/nfs4filelayout.c | 15 +++++++++++++--
> fs/nfs/objlayout/objio_osd.c | 13 ++++++++++++-
> fs/nfs/pagelist.c | 12 +++++-------
> fs/nfs/pnfs.h | 29 ++++++++++++++++++++++-------
> fs/nfs/read.c | 42 +++++++++++++++++++++++++++++++-----------
> fs/nfs/write.c | 27 +++++++++++++++++++++++----
> include/linux/nfs_page.h | 17 ++++++++++++++---
> 7 files changed, 120 insertions(+), 35 deletions(-)
>
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index 4269088..e9b9b82 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -654,7 +654,7 @@ filelayout_alloc_lseg(struct pnfs_layout_hdr *layoutid,
> * return true : coalesce page
> * return false : don't coalesce page
> */
> -bool
> +static bool
> filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
> struct nfs_page *req)
> {
> @@ -676,6 +676,16 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
> return (p_stripe == r_stripe);
> }
>
> +static const struct nfs_pageio_ops filelayout_pg_read_ops = {
> + .pg_test = filelayout_pg_test,
> + .pg_doio = nfs_generic_pg_readpages,
> +};
> +
> +static const struct nfs_pageio_ops filelayout_pg_write_ops = {
> + .pg_test = filelayout_pg_test,
> + .pg_doio = nfs_generic_pg_writepages,
> +};
> +
> static bool filelayout_mark_pnfs_commit(struct pnfs_layout_segment *lseg)
> {
> return !FILELAYOUT_LSEG(lseg)->commit_through_mds;
> @@ -873,7 +883,8 @@ static struct pnfs_layoutdriver_type filelayout_type = {
> .owner = THIS_MODULE,
> .alloc_lseg = filelayout_alloc_lseg,
> .free_lseg = filelayout_free_lseg,
> - .pg_test = filelayout_pg_test,
> + .pg_read_ops = &filelayout_pg_read_ops,
> + .pg_write_ops = &filelayout_pg_write_ops,
> .mark_pnfs_commit = filelayout_mark_pnfs_commit,
> .choose_commit_list = filelayout_choose_commit_list,
> .commit_pagelist = filelayout_commit_pagelist,
> diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
> index 4c41a60..31088f3 100644
> --- a/fs/nfs/objlayout/objio_osd.c
> +++ b/fs/nfs/objlayout/objio_osd.c
> @@ -1008,6 +1008,16 @@ static bool objio_pg_test(struct nfs_pageio_descriptor *pgio,
> OBJIO_LSEG(pgio->pg_lseg)->max_io_size;
> }
>
> +static const struct nfs_pageio_ops objio_pg_read_ops = {
> + .pg_test = objio_pg_test,
> + .pg_doio = nfs_generic_pg_readpages,
> +};
> +
> +static const struct nfs_pageio_ops objio_pg_write_ops = {
> + .pg_test = objio_pg_test,
> + .pg_doio = nfs_generic_pg_writepages,
Hmm, for the non-files layouts we should ignore the server's wsize
for pnfs I/O.
How about using the follwing?
int pnfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
{
return desc->pg_lseg ? nfs_pagein_one(desc) : nfs_generic_pg_readpages(desc);
}
EXPORT_SYMBOL_GPL(pnfs_generic_pg_readpages);
int pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
{
return desc->pg_lseg ? nfs_flush_one(desc) : nfs_generic_pg_writepages(desc);
}
EXPORT_SYMBOL_GPL(pnfs_generic_pg_writepages);
Benny
> +};
> +
> static struct pnfs_layoutdriver_type objlayout_type = {
> .id = LAYOUT_OSD2_OBJECTS,
> .name = "LAYOUT_OSD2_OBJECTS",
> @@ -1021,7 +1031,8 @@ static struct pnfs_layoutdriver_type objlayout_type = {
>
> .read_pagelist = objlayout_read_pagelist,
> .write_pagelist = objlayout_write_pagelist,
> - .pg_test = objio_pg_test,
> + .pg_read_ops = &objio_pg_read_ops,
> + .pg_write_ops = &objio_pg_write_ops,
>
> .free_deviceid_node = objio_free_deviceid_node,
>
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index 7913961..2b71ad0 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -204,7 +204,7 @@ nfs_wait_on_request(struct nfs_page *req)
> TASK_UNINTERRUPTIBLE);
> }
>
> -static bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc, struct nfs_page *prev, struct nfs_page *req)
> +bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc, struct nfs_page *prev, struct nfs_page *req)
> {
> /*
> * FIXME: ideally we should be able to coalesce all requests
> @@ -229,7 +229,7 @@ static bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc, struct nfs_p
> */
> void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> struct inode *inode,
> - int (*doio)(struct nfs_pageio_descriptor *),
> + const struct nfs_pageio_ops *pg_ops,
> size_t bsize,
> int io_flags)
> {
> @@ -240,12 +240,10 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> desc->pg_base = 0;
> desc->pg_moreio = 0;
> desc->pg_inode = inode;
> - desc->pg_doio = doio;
> + desc->pg_ops = pg_ops;
> desc->pg_ioflags = io_flags;
> desc->pg_error = 0;
> desc->pg_lseg = NULL;
> - desc->pg_test = nfs_generic_pg_test;
> - pnfs_pageio_init(desc, inode);
> }
>
> /**
> @@ -275,7 +273,7 @@ static bool nfs_can_coalesce_requests(struct nfs_page *prev,
> return false;
> if (prev->wb_pgbase + prev->wb_bytes != PAGE_CACHE_SIZE)
> return false;
> - return pgio->pg_test(pgio, prev, req);
> + return pgio->pg_ops->pg_test(pgio, prev, req);
> }
>
> /**
> @@ -310,7 +308,7 @@ static int nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc,
> static void nfs_pageio_doio(struct nfs_pageio_descriptor *desc)
> {
> if (!list_empty(&desc->pg_list)) {
> - int error = desc->pg_doio(desc);
> + int error = desc->pg_ops->pg_doio(desc);
> if (error < 0)
> desc->pg_error = error;
> else
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 48d0a8e..8d063c0 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -87,7 +87,8 @@ struct pnfs_layoutdriver_type {
> void (*free_lseg) (struct pnfs_layout_segment *lseg);
>
> /* test for nfs page cache coalescing */
> - bool (*pg_test)(struct nfs_pageio_descriptor *, struct nfs_page *, struct nfs_page *);
> + const struct nfs_pageio_ops *pg_read_ops;
> + const struct nfs_pageio_ops *pg_write_ops;
>
> /* Returns true if layoutdriver wants to divert this request to
> * driver's commit routine.
> @@ -292,13 +293,22 @@ static inline int pnfs_return_layout(struct inode *ino)
> return 0;
> }
>
> -static inline void pnfs_pageio_init(struct nfs_pageio_descriptor *pgio,
> - struct inode *inode)
> +static inline const struct nfs_pageio_ops *pnfs_get_read_ops(struct inode *inode)
> {
> struct pnfs_layoutdriver_type *ld = NFS_SERVER(inode)->pnfs_curr_ld;
>
> - if (ld)
> - pgio->pg_test = ld->pg_test;
> + if (!ld)
> + return NULL;
> + return ld->pg_read_ops;
> +}
> +
> +static inline const struct nfs_pageio_ops *pnfs_get_write_ops(struct inode *inode)
> +{
> + struct pnfs_layoutdriver_type *ld = NFS_SERVER(inode)->pnfs_curr_ld;
> +
> + if (!ld)
> + return NULL;
> + return ld->pg_write_ops;
> }
>
> #else /* CONFIG_NFS_V4_1 */
> @@ -384,9 +394,14 @@ static inline void unset_pnfs_layoutdriver(struct nfs_server *s)
> {
> }
>
> -static inline void pnfs_pageio_init(struct nfs_pageio_descriptor *pgio,
> - struct inode *inode)
> +static inline const struct nfs_pageio_ops *pnfs_get_read_ops(struct inode *inode)
> {
> + return NULL;
> +}
> +
> +static inline const struct nfs_pageio_ops *pnfs_get_write_ops(struct inode *inode)
> +{
> + return NULL;
> }
>
> static inline void
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 20a7f95..b0f5c19 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -32,6 +32,7 @@
>
> static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc);
> static int nfs_pagein_one(struct nfs_pageio_descriptor *desc);
> +static const struct nfs_pageio_ops nfs_pageio_read_ops;
> static const struct rpc_call_ops nfs_read_partial_ops;
> static const struct rpc_call_ops nfs_read_full_ops;
>
> @@ -113,6 +114,19 @@ static void nfs_readpage_truncate_uninitialised_page(struct nfs_read_data *data)
> }
> }
>
> +static void nfs_pageio_init_read(struct nfs_pageio_descriptor *pgio,
> + struct inode *inode)
> +{
> + size_t rsize = NFS_SERVER(inode)->rsize;
> + const struct nfs_pageio_ops *ops;
> +
> + ops = pnfs_get_read_ops(inode);
> + if (ops == NULL)
> + ops = &nfs_pageio_read_ops;
> +
> + nfs_pageio_init(pgio, inode, ops, rsize, 0);
> +}
> +
> int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
> struct page *page)
> {
> @@ -131,14 +145,11 @@ int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
> if (len < PAGE_CACHE_SIZE)
> zero_user_segment(page, len, PAGE_CACHE_SIZE);
>
> - nfs_pageio_init(&pgio, inode, NULL, 0, 0);
> + nfs_pageio_init_read(&pgio, inode);
> nfs_list_add_request(new, &pgio.pg_list);
> pgio.pg_count = len;
>
> - if (NFS_SERVER(inode)->rsize < PAGE_CACHE_SIZE)
> - nfs_pagein_multi(&pgio);
> - else
> - nfs_pagein_one(&pgio);
> + nfs_pageio_complete(&pgio);
> return 0;
> }
>
> @@ -365,6 +376,20 @@ out:
> return ret;
> }
>
> +int nfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc)
> +{
> + if (desc->pg_bsize < PAGE_CACHE_SIZE)
> + return nfs_pagein_multi(desc);
> + return nfs_pagein_one(desc);
> +}
> +EXPORT_SYMBOL_GPL(nfs_generic_pg_readpages);
> +
> +
> +static const struct nfs_pageio_ops nfs_pageio_read_ops = {
> + .pg_test = nfs_generic_pg_test,
> + .pg_doio = nfs_generic_pg_readpages,
> +};
> +
> /*
> * This is the callback from RPC telling us whether a reply was
> * received or some error occurred (timeout or socket shutdown).
> @@ -635,8 +660,6 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
> .pgio = &pgio,
> };
> struct inode *inode = mapping->host;
> - struct nfs_server *server = NFS_SERVER(inode);
> - size_t rsize = server->rsize;
> unsigned long npages;
> int ret = -ESTALE;
>
> @@ -664,10 +687,7 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
> if (ret == 0)
> goto read_complete; /* all pages were read */
>
> - if (rsize < PAGE_CACHE_SIZE)
> - nfs_pageio_init(&pgio, inode, nfs_pagein_multi, rsize, 0);
> - else
> - nfs_pageio_init(&pgio, inode, nfs_pagein_one, rsize, 0);
> + nfs_pageio_init_read(&pgio, inode);
>
> ret = read_cache_pages(mapping, pages, readpage_async_filler, &desc);
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index e268e3b..c379c86 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -977,6 +977,11 @@ out_bad:
> return -ENOMEM;
> }
>
> +static const struct nfs_pageio_ops nfs_flush_multi_ops = {
> + .pg_test = nfs_generic_pg_test,
> + .pg_doio = nfs_flush_multi,
> +};
> +
> /*
> * Create an RPC task for the given write request and kick it.
> * The page must have been locked by the caller.
> @@ -1031,15 +1036,29 @@ out:
> return ret;
> }
>
> +int nfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
> +{
> + if (desc->pg_bsize < PAGE_CACHE_SIZE)
> + return nfs_flush_multi(desc);
> + return nfs_flush_one(desc);
> +}
> +EXPORT_SYMBOL_GPL(nfs_generic_pg_writepages);
> +
> +static const struct nfs_pageio_ops nfs_pageio_write_ops = {
> + .pg_test = nfs_generic_pg_test,
> + .pg_doio = nfs_generic_pg_writepages,
> +};
> +
> static void nfs_pageio_init_write(struct nfs_pageio_descriptor *pgio,
> struct inode *inode, int ioflags)
> {
> size_t wsize = NFS_SERVER(inode)->wsize;
> + const struct nfs_pageio_ops *ops;
> + ops = pnfs_get_write_ops(inode);
> + if (ops == NULL)
> + ops = &nfs_pageio_write_ops;
>
> - if (wsize < PAGE_CACHE_SIZE)
> - nfs_pageio_init(pgio, inode, nfs_flush_multi, wsize, ioflags);
> - else
> - nfs_pageio_init(pgio, inode, nfs_flush_one, wsize, ioflags);
> + nfs_pageio_init(pgio, inode, &nfs_pageio_write_ops, wsize, ioflags);
> }
>
> /*
> diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
> index 3a34e80..b0e26c0 100644
> --- a/include/linux/nfs_page.h
> +++ b/include/linux/nfs_page.h
> @@ -55,6 +55,12 @@ struct nfs_page {
> struct nfs_writeverf wb_verf; /* Commit cookie */
> };
>
> +struct nfs_pageio_descriptor;
> +struct nfs_pageio_ops {
> + bool (*pg_test)(struct nfs_pageio_descriptor *, struct nfs_page *, struct nfs_page *);
> + int (*pg_doio)(struct nfs_pageio_descriptor *);
> +};
> +
> struct nfs_pageio_descriptor {
> struct list_head pg_list;
> unsigned long pg_bytes_written;
> @@ -64,11 +70,10 @@ struct nfs_pageio_descriptor {
> char pg_moreio;
>
> struct inode *pg_inode;
> - int (*pg_doio)(struct nfs_pageio_descriptor *);
> + const struct nfs_pageio_ops *pg_ops;
> int pg_ioflags;
> int pg_error;
> struct pnfs_layout_segment *pg_lseg;
> - bool (*pg_test)(struct nfs_pageio_descriptor *, struct nfs_page *, struct nfs_page *);
> };
>
> #define NFS_WBACK_BUSY(req) (test_bit(PG_BUSY,&(req)->wb_flags))
> @@ -85,18 +90,24 @@ extern int nfs_scan_list(struct nfs_inode *nfsi, struct list_head *dst,
> pgoff_t idx_start, unsigned int npages, int tag);
> extern void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> struct inode *inode,
> - int (*doio)(struct nfs_pageio_descriptor *desc),
> + const struct nfs_pageio_ops *pg_ops,
> size_t bsize,
> int how);
> extern int nfs_pageio_add_request(struct nfs_pageio_descriptor *,
> struct nfs_page *);
> extern void nfs_pageio_complete(struct nfs_pageio_descriptor *desc);
> extern void nfs_pageio_cond_complete(struct nfs_pageio_descriptor *, pgoff_t);
> +extern bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc,
> + struct nfs_page *prev,
> + struct nfs_page *req);
> extern int nfs_wait_on_request(struct nfs_page *);
> extern void nfs_unlock_request(struct nfs_page *req);
> extern int nfs_set_page_tag_locked(struct nfs_page *req);
> extern void nfs_clear_page_tag_locked(struct nfs_page *req);
>
> +extern int nfs_generic_pg_readpages(struct nfs_pageio_descriptor *desc);
> +extern int nfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc);
> +
>
> /*
> * Lock the page of an asynchronous request without getting a new reference
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
