From: Benny Halevy <bhalevy@panasas.com>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/3] NFS: Cleanup of the nfs_pageio code in preparation for a pnfs bugfix
Date: Tue, 07 Jun 2011 20:28:17 -0400 [thread overview]
Message-ID: <4DEEC221.6040902@panasas.com> (raw)
In-Reply-To: <1307399551-17489-2-git-send-email-Trond.Myklebust@netapp.com>
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@netapp.com>
> ---
> 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,
> +};
> +
> 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);
Trond, this should be:
nfs_pageio_init(pgio, inode, ops, wsize, ioflags);
Benny
> }
>
> /*
> 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
next prev parent reply other threads:[~2011-06-08 0:28 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-06 22:32 [PATCH 1/3] NFSv4.1: Fix some issues with pnfs_generic_pg_test Trond Myklebust
2011-06-06 22:32 ` [PATCH 2/3] NFS: Cleanup of the nfs_pageio code in preparation for a pnfs bugfix Trond Myklebust
2011-06-06 22:32 ` [PATCH 3/3] NFSv4.1: Add an initialisation callback for pNFS Trond Myklebust
2011-06-08 0:28 ` Benny Halevy [this message]
2011-06-08 0:51 ` [PATCH 2/3] NFS: Cleanup of the nfs_pageio code in preparation for a pnfs bugfix Trond Myklebust
2011-06-08 2:30 ` Benny Halevy
2011-06-09 16:37 ` Trond Myklebust
2011-06-09 17:51 ` Boaz Harrosh
2011-06-09 18:13 ` Trond Myklebust
2011-06-09 18:58 ` Benny Halevy
2011-06-09 21:31 ` Boaz Harrosh
2011-06-08 0:53 ` [PATCH 1/3] NFSv4.1: Fix some issues with pnfs_generic_pg_test Benny Halevy
2011-06-08 1:12 ` Trond Myklebust
2011-06-08 2:24 ` Benny Halevy
2011-06-09 16:31 ` Trond Myklebust
2011-06-09 18:43 ` Benny Halevy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DEEC221.6040902@panasas.com \
--to=bhalevy@panasas.com \
--cc=Trond.Myklebust@netapp.com \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.