From: Benny Halevy <bhalevy@panasas.com>
To: Fred Isaman <iisaman@netapp.com>
Cc: linux-nfs@vger.kernel.org, Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Re: [PATCH 8/9] NFSv4.1: rearrange ->doio args
Date: Wed, 09 Mar 2011 23:58:06 -0800 [thread overview]
Message-ID: <4D78848E.1050902@panasas.com> (raw)
In-Reply-To: <1299165229-3148-9-git-send-email-iisaman@netapp.com>
On 2011-03-03 07:13, Fred Isaman wrote:
> This will make it possible to clear the lseg pointer in the same
> function as it is put, instead of in the caller nfs_pageio_doio().
so much better this way :)
I'm glad we discussed this in the Connectathon!
Benny
>
> Signed-off-by: Fred Isaman <iisaman@netapp.com>
> ---
> fs/nfs/pagelist.c | 10 ++--------
> fs/nfs/read.c | 42 +++++++++++++++++++++++++-----------------
> fs/nfs/write.c | 28 ++++++++++++++++------------
> include/linux/nfs_page.h | 4 ++--
> 4 files changed, 45 insertions(+), 39 deletions(-)
>
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index 45b0fb8..9f62874 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -214,7 +214,7 @@ nfs_wait_on_request(struct nfs_page *req)
> */
> void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> struct inode *inode,
> - int (*doio)(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *),
> + int (*doio)(struct nfs_pageio_descriptor *),
> size_t bsize,
> int io_flags)
> {
> @@ -311,13 +311,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->pg_inode,
> - &desc->pg_list,
> - nfs_page_array_len(desc->pg_base,
> - desc->pg_count),
> - desc->pg_count,
> - desc->pg_ioflags,
> - desc->pg_lseg);
> + int error = desc->pg_doio(desc);
> desc->pg_lseg = NULL;
> if (error < 0)
> desc->pg_error = error;
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index f40c7f4..ab9c776 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -31,8 +31,8 @@
>
> #define NFSDBG_FACILITY NFSDBG_PAGECACHE
>
> -static int nfs_pagein_multi(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *);
> -static int nfs_pagein_one(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *);
> +static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc);
> +static int nfs_pagein_one(struct nfs_pageio_descriptor *desc);
> static const struct rpc_call_ops nfs_read_partial_ops;
> static const struct rpc_call_ops nfs_read_full_ops;
>
> @@ -117,9 +117,9 @@ static void nfs_readpage_truncate_uninitialised_page(struct nfs_read_data *data)
> int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
> struct page *page)
> {
> - LIST_HEAD(one_request);
> struct nfs_page *new;
> unsigned int len;
> + struct nfs_pageio_descriptor pgio;
>
> len = nfs_page_length(page);
> if (len == 0)
> @@ -132,11 +132,14 @@ 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_list_add_request(new, &one_request);
> + nfs_pageio_init(&pgio, inode, NULL, 0, 0);
> + nfs_list_add_request(new, &pgio.pg_list);
> + pgio.pg_count = len;
> +
> if (NFS_SERVER(inode)->rsize < PAGE_CACHE_SIZE)
> - nfs_pagein_multi(inode, &one_request, 1, len, 0, NULL);
> + nfs_pagein_multi(&pgio);
> else
> - nfs_pagein_one(inode, &one_request, 1, len, 0, NULL);
> + nfs_pagein_one(&pgio);
> return 0;
> }
>
> @@ -258,20 +261,21 @@ nfs_async_read_error(struct list_head *head)
> * won't see the new data until our attribute cache is updated. This is more
> * or less conventional NFS client behavior.
> */
> -static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int flags, struct pnfs_layout_segment *lseg)
> +static int nfs_pagein_multi(struct nfs_pageio_descriptor *desc)
> {
> - struct nfs_page *req = nfs_list_entry(head->next);
> + struct nfs_page *req = nfs_list_entry(desc->pg_list.next);
> struct page *page = req->wb_page;
> struct nfs_read_data *data;
> - size_t rsize = NFS_SERVER(inode)->rsize, nbytes;
> + size_t rsize = NFS_SERVER(desc->pg_inode)->rsize, nbytes;
> unsigned int offset;
> int requests = 0;
> int ret = 0;
> + struct pnfs_layout_segment *lseg;
> LIST_HEAD(list);
>
> nfs_list_remove_request(req);
>
> - nbytes = count;
> + nbytes = desc->pg_count;
> do {
> size_t len = min(nbytes,rsize);
>
> @@ -284,11 +288,11 @@ static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigne
> } while(nbytes != 0);
> atomic_set(&req->wb_complete, requests);
>
> - /* We know lseg==NULL */
> - lseg = pnfs_update_layout(inode, req->wb_context, IOMODE_READ);
> + BUG_ON(desc->pg_lseg != NULL);
> + lseg = pnfs_update_layout(desc->pg_inode, req->wb_context, IOMODE_READ);
> ClearPageError(page);
> offset = 0;
> - nbytes = count;
> + nbytes = desc->pg_count;
> do {
> int ret2;
>
> @@ -321,14 +325,17 @@ out_bad:
> return -ENOMEM;
> }
>
> -static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int flags, struct pnfs_layout_segment *lseg)
> +static int nfs_pagein_one(struct nfs_pageio_descriptor *desc)
> {
> struct nfs_page *req;
> struct page **pages;
> struct nfs_read_data *data;
> + struct list_head *head = &desc->pg_list;
> + struct pnfs_layout_segment *lseg = desc->pg_lseg;
> int ret = -ENOMEM;
>
> - data = nfs_readdata_alloc(npages);
> + data = nfs_readdata_alloc(nfs_page_array_len(desc->pg_base,
> + desc->pg_count));
> if (!data) {
> nfs_async_read_error(head);
> goto out;
> @@ -344,9 +351,10 @@ static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned
> }
> req = nfs_list_entry(data->pages.next);
> if ((!lseg) && list_is_singular(&data->pages))
> - lseg = pnfs_update_layout(inode, req->wb_context, IOMODE_READ);
> + lseg = pnfs_update_layout(desc->pg_inode, req->wb_context, IOMODE_READ);
>
> - ret = nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0, lseg);
> + ret = nfs_read_rpcsetup(req, data, &nfs_read_full_ops, desc->pg_count,
> + 0, lseg);
> out:
> put_lseg(lseg);
> return ret;
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 06a1f3f..ccc7c22 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -898,20 +898,21 @@ static void nfs_redirty_request(struct nfs_page *req)
> * Generate multiple small requests to write out a single
> * contiguous dirty area on one page.
> */
> -static int nfs_flush_multi(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int how, struct pnfs_layout_segment *lseg)
> +static int nfs_flush_multi(struct nfs_pageio_descriptor *desc)
> {
> - struct nfs_page *req = nfs_list_entry(head->next);
> + struct nfs_page *req = nfs_list_entry(desc->pg_list.next);
> struct page *page = req->wb_page;
> struct nfs_write_data *data;
> - size_t wsize = NFS_SERVER(inode)->wsize, nbytes;
> + size_t wsize = NFS_SERVER(desc->pg_inode)->wsize, nbytes;
> unsigned int offset;
> int requests = 0;
> int ret = 0;
> + struct pnfs_layout_segment *lseg;
> LIST_HEAD(list);
>
> nfs_list_remove_request(req);
>
> - nbytes = count;
> + nbytes = desc->pg_count;
> do {
> size_t len = min(nbytes, wsize);
>
> @@ -924,11 +925,11 @@ static int nfs_flush_multi(struct inode *inode, struct list_head *head, unsigned
> } while (nbytes != 0);
> atomic_set(&req->wb_complete, requests);
>
> - BUG_ON(lseg);
> - lseg = pnfs_update_layout(inode, req->wb_context, IOMODE_RW);
> + BUG_ON(desc->pg_lseg);
> + lseg = pnfs_update_layout(desc->pg_inode, req->wb_context, IOMODE_RW);
> ClearPageError(page);
> offset = 0;
> - nbytes = count;
> + nbytes = desc->pg_count;
> do {
> int ret2;
>
> @@ -940,7 +941,7 @@ static int nfs_flush_multi(struct inode *inode, struct list_head *head, unsigned
> if (nbytes < wsize)
> wsize = nbytes;
> ret2 = nfs_write_rpcsetup(req, data, &nfs_write_partial_ops,
> - wsize, offset, lseg, how);
> + wsize, offset, lseg, desc->pg_ioflags);
> if (ret == 0)
> ret = ret2;
> offset += wsize;
> @@ -968,14 +969,17 @@ out_bad:
> * This is the case if nfs_updatepage detects a conflicting request
> * that has been written but not committed.
> */
> -static int nfs_flush_one(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int how, struct pnfs_layout_segment *lseg)
> +static int nfs_flush_one(struct nfs_pageio_descriptor *desc)
> {
> struct nfs_page *req;
> struct page **pages;
> struct nfs_write_data *data;
> + struct list_head *head = &desc->pg_list;
> + struct pnfs_layout_segment *lseg = desc->pg_lseg;
> int ret;
>
> - data = nfs_writedata_alloc(npages);
> + data = nfs_writedata_alloc(nfs_page_array_len(desc->pg_base,
> + desc->pg_count));
> if (!data) {
> while (!list_empty(head)) {
> req = nfs_list_entry(head->next);
> @@ -995,10 +999,10 @@ static int nfs_flush_one(struct inode *inode, struct list_head *head, unsigned i
> }
> req = nfs_list_entry(data->pages.next);
> if ((!lseg) && list_is_singular(&data->pages))
> - lseg = pnfs_update_layout(inode, req->wb_context, IOMODE_RW);
> + lseg = pnfs_update_layout(desc->pg_inode, req->wb_context, IOMODE_RW);
>
> /* Set up the argument struct */
> - ret = nfs_write_rpcsetup(req, data, &nfs_write_full_ops, count, 0, lseg, how);
> + ret = nfs_write_rpcsetup(req, data, &nfs_write_full_ops, desc->pg_count, 0, lseg, desc->pg_ioflags);
> out:
> put_lseg(lseg); /* Cleans any gotten in ->pg_test */
> return ret;
> diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
> index ba88ff4..90907ad 100644
> --- a/include/linux/nfs_page.h
> +++ b/include/linux/nfs_page.h
> @@ -59,7 +59,7 @@ struct nfs_pageio_descriptor {
> unsigned int pg_base;
>
> struct inode *pg_inode;
> - int (*pg_doio)(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *);
> + int (*pg_doio)(struct nfs_pageio_descriptor *);
> int pg_ioflags;
> int pg_error;
> struct pnfs_layout_segment *pg_lseg;
> @@ -81,7 +81,7 @@ 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 inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *),
> + int (*doio)(struct nfs_pageio_descriptor *desc),
> size_t bsize,
> int how);
> extern int nfs_pageio_add_request(struct nfs_pageio_descriptor *,
next prev parent reply other threads:[~2011-03-10 7:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-03 15:13 [PATCH 0/9] NFSv4.1: pnfs wave4 submission v2 Fred Isaman
2011-03-03 15:13 ` [PATCH 1/9] NFSv4.1: rearrange nfs_write_rpcsetup Fred Isaman
2011-03-03 15:13 ` [PATCH 2/9] NFSv4.1: add callback to nfs4_write_done Fred Isaman
2011-03-03 15:13 ` [PATCH 3/9] NFSv4.1: Send lseg down into nfs_write_rpcsetup Fred Isaman
2011-03-03 15:13 ` [PATCH 4/9] NFSv4.1: trigger LAYOUTGET for writes Fred Isaman
2011-03-03 15:13 ` [PATCH 5/9] NFSv4.1: implement generic pnfs layer write switch Fred Isaman
2011-03-03 15:13 ` [PATCH 6/9] NFSv4.1: remove GETATTR from ds writes Fred Isaman
2011-03-03 15:13 ` [PATCH 7/9] NFSv4.1: pnfs filelayout driver write Fred Isaman
2011-03-03 15:13 ` [PATCH 8/9] NFSv4.1: rearrange ->doio args Fred Isaman
2011-03-10 7:58 ` Benny Halevy [this message]
2011-03-03 15:13 ` [PATCH 9/9] NFSv4.1: Clear lseg pointer in ->doio function Fred Isaman
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=4D78848E.1050902@panasas.com \
--to=bhalevy@panasas.com \
--cc=Trond.Myklebust@netapp.com \
--cc=iisaman@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.