From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: andros@netapp.com
Cc: linux-nfs@vger.kernel.org, Fred Isaman <iisaman@netapp.com>,
Andy Adamon <andros@citi.umich.edu>,
Dean Hildebrand <dhildeb@us.ibm.com>,
Fred Isaman <iisaman@citi.umich.edu>,
Benny Halevy <bhalevy@panasas.com>,
Boaz Harrosh <bharrosh@panasas.com>,
Oleg Drokin <green@linuxhacker.ru>, Tao Guo <guotao@nrchpc.ac.cn>
Subject: Re: [PATCH 09/16] pnfs: wave 3: shift pnfs_update_layout locations
Date: Mon, 14 Feb 2011 18:14:12 -0500 [thread overview]
Message-ID: <1297725252.23841.45.camel@heimdal.trondhjem.org> (raw)
In-Reply-To: <1297711116-3139-10-git-send-email-andros@netapp.com>
On Mon, 2011-02-14 at 14:18 -0500, andros@netapp.com wrote:
> From: Fred Isaman <iisaman@netapp.com>
>
> Move the pnfs_update_layout call location to nfs_pageio_do_add_request().
> Grab the lseg sent in the doio function to nfs_read_rpcsetup and attach
> it to each nfs_read_data so it can be sent to the layout driver.
>
> Signed-off-by: Andy Adamon <andros@netapp.com>
> Signed-off-by: Andy Adamon <andros@citi.umich.edu>
> Signed-off-by: Dean Hildebrand <dhildeb@us.ibm.com>
> Signed-off-by: Fred Isaman <iisaman@citi.umich.edu>
> Signed-off-by: Fred Isaman <iisaman@netapp.com>
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
> Signed-off-by: Tao Guo <guotao@nrchpc.ac.cn>
> ---
> fs/nfs/file.c | 4 ----
> fs/nfs/pagelist.c | 15 ++++++++++++---
> fs/nfs/pnfs.c | 4 ++--
> fs/nfs/pnfs.h | 1 +
> fs/nfs/read.c | 28 ++++++++++++++++------------
> fs/nfs/write.c | 4 ++--
> include/linux/nfs_page.h | 5 +++--
> include/linux/nfs_xdr.h | 1 +
> 8 files changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 7bf029e..d85a534 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -387,10 +387,6 @@ static int nfs_write_begin(struct file *file, struct address_space *mapping,
> file->f_path.dentry->d_name.name,
> mapping->host->i_ino, len, (long long) pos);
>
> - pnfs_update_layout(mapping->host,
> - nfs_file_open_context(file),
> - IOMODE_RW);
> -
> start:
> /*
> * Prevent starvation issues if someone is doing a consistency
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index e1164e3..e0a0cb4 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -20,6 +20,7 @@
> #include <linux/nfs_mount.h>
>
> #include "internal.h"
> +#include "pnfs.h"
>
> static struct kmem_cache *nfs_page_cachep;
>
> @@ -213,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),
> + int (*doio)(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *),
> size_t bsize,
> int io_flags)
> {
> @@ -226,6 +227,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> desc->pg_doio = doio;
> desc->pg_ioflags = io_flags;
> desc->pg_error = 0;
> + desc->pg_lseg = NULL;
> }
>
> /**
> @@ -288,8 +290,13 @@ static int nfs_pageio_do_add_request(struct nfs_pageio_descriptor *desc,
> prev = nfs_list_entry(desc->pg_list.prev);
> if (!nfs_can_coalesce_requests(prev, req))
> return 0;
> - } else
> + } else {
> + put_lseg(desc->pg_lseg);
> desc->pg_base = req->wb_pgbase;
> + desc->pg_lseg = pnfs_update_layout(desc->pg_inode,
> + req->wb_context,
> + IOMODE_READ);
Looking at this afresh after a week of vacation. Isn't it more natural
to do this as part of the pg_doio() callback?
Your only reason for introducing the ->pg_lseg pointer is to be able to
pass it to the ->pg_doio() in the first place. Why not do that by simply
passing the 'desc' pointer to ->pg_doio(), and then having it call
pnfs_update_layout() instead of 'get_layout()'?
> + }
> nfs_list_remove_request(req);
> nfs_list_add_request(req, &desc->pg_list);
> desc->pg_count = newlen;
> @@ -307,7 +314,8 @@ static void nfs_pageio_doio(struct nfs_pageio_descriptor *desc)
> nfs_page_array_len(desc->pg_base,
> desc->pg_count),
> desc->pg_count,
> - desc->pg_ioflags);
> + desc->pg_ioflags,
> + desc->pg_lseg);
> if (error < 0)
> desc->pg_error = error;
> else
> @@ -345,6 +353,7 @@ int nfs_pageio_add_request(struct nfs_pageio_descriptor *desc,
> void nfs_pageio_complete(struct nfs_pageio_descriptor *desc)
> {
> nfs_pageio_doio(desc);
> + put_lseg(desc->pg_lseg);
> }
>
> /**
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index f0a9578..dcd4356 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -264,7 +264,7 @@ put_lseg_locked(struct pnfs_layout_segment *lseg,
> return 0;
> }
>
> -static void
> +void
> put_lseg(struct pnfs_layout_segment *lseg)
> {
> struct inode *ino;
> @@ -285,6 +285,7 @@ put_lseg(struct pnfs_layout_segment *lseg)
> pnfs_free_lseg_list(&free_me);
> }
> }
> +EXPORT_SYMBOL_GPL(put_lseg);
Why is this needed here?
> static bool
> should_free_lseg(u32 lseg_iomode, u32 recall_iomode)
> @@ -797,7 +798,6 @@ pnfs_update_layout(struct inode *ino,
> out:
> dprintk("%s end, state 0x%lx lseg %p\n", __func__,
> nfsi->layout ? nfsi->layout->plh_flags : -1, lseg);
> - put_lseg(lseg); /* STUB - callers currently ignore return value */
> return lseg;
> out_unlock:
> spin_unlock(&ino->i_lock);
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 9a994bc..121d6a3 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -146,6 +146,7 @@ extern int nfs4_proc_layoutget(struct nfs4_layoutget *lgp);
>
> /* pnfs.c */
> void get_layout_hdr(struct pnfs_layout_hdr *lo);
> +void put_lseg(struct pnfs_layout_segment *lseg);
> struct pnfs_layout_segment *
> pnfs_update_layout(struct inode *ino, struct nfs_open_context *ctx,
> enum pnfs_iomode access_type);
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index aedcaa7..c453164 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -20,17 +20,17 @@
> #include <linux/nfs_page.h>
>
> #include <asm/system.h>
> +#include "pnfs.h"
>
> #include "nfs4_fs.h"
> #include "internal.h"
> #include "iostat.h"
> #include "fscache.h"
> -#include "pnfs.h"
>
> #define NFSDBG_FACILITY NFSDBG_PAGECACHE
>
> -static int nfs_pagein_multi(struct inode *, struct list_head *, unsigned int, size_t, int);
> -static int nfs_pagein_one(struct inode *, struct list_head *, unsigned int, size_t, int);
> +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 const struct rpc_call_ops nfs_read_partial_ops;
> static const struct rpc_call_ops nfs_read_full_ops;
>
> @@ -70,6 +70,7 @@ void nfs_readdata_free(struct nfs_read_data *p)
> static void nfs_readdata_release(struct nfs_read_data *rdata)
> {
> put_nfs_open_context(rdata->args.context);
> + put_lseg(rdata->lseg);
Shouldn't you be calling put_lseg() _before_ put_nfs_open_context()? You
are not guaranteed that the inode still exists after that call.
> nfs_readdata_free(rdata);
> }
>
> @@ -117,11 +118,11 @@ int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
> LIST_HEAD(one_request);
> struct nfs_page *new;
> unsigned int len;
> + struct pnfs_layout_segment *lseg;
>
> len = nfs_page_length(page);
> if (len == 0)
> return nfs_return_empty_page(page);
> - pnfs_update_layout(inode, ctx, IOMODE_READ);
> new = nfs_create_request(ctx, inode, page, 0, len);
> if (IS_ERR(new)) {
> unlock_page(page);
> @@ -131,10 +132,12 @@ int nfs_readpage_async(struct nfs_open_context *ctx, struct inode *inode,
> zero_user_segment(page, len, PAGE_CACHE_SIZE);
>
> nfs_list_add_request(new, &one_request);
> + lseg = pnfs_update_layout(inode, ctx, IOMODE_READ);
> if (NFS_SERVER(inode)->rsize < PAGE_CACHE_SIZE)
> - nfs_pagein_multi(inode, &one_request, 1, len, 0);
> + nfs_pagein_multi(inode, &one_request, 1, len, 0, lseg);
> else
> - nfs_pagein_one(inode, &one_request, 1, len, 0);
> + nfs_pagein_one(inode, &one_request, 1, len, 0, lseg);
> + put_lseg(lseg);
> return 0;
> }
>
> @@ -160,7 +163,8 @@ static void nfs_readpage_release(struct nfs_page *req)
> */
> static int nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
> const struct rpc_call_ops *call_ops,
> - unsigned int count, unsigned int offset)
> + unsigned int count, unsigned int offset,
> + struct pnfs_layout_segment *lseg)
> {
> struct inode *inode = req->wb_context->path.dentry->d_inode;
> int swap_flags = IS_SWAPFILE(inode) ? NFS_RPC_SWAPFLAGS : 0;
> @@ -183,6 +187,7 @@ static int nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
> data->req = req;
> data->inode = inode;
> data->cred = msg.rpc_cred;
> + data->lseg = get_lseg(lseg);
>
> data->args.fh = NFS_FH(inode);
> data->args.offset = req_offset(req) + offset;
> @@ -240,7 +245,7 @@ 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)
> +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)
> {
> struct nfs_page *req = nfs_list_entry(head->next);
> struct page *page = req->wb_page;
> @@ -280,7 +285,7 @@ static int nfs_pagein_multi(struct inode *inode, struct list_head *head, unsigne
> if (nbytes < rsize)
> rsize = nbytes;
> ret2 = nfs_read_rpcsetup(req, data, &nfs_read_partial_ops,
> - rsize, offset);
> + rsize, offset, lseg);
> if (ret == 0)
> ret = ret2;
> offset += rsize;
> @@ -300,7 +305,7 @@ out_bad:
> return -ENOMEM;
> }
>
> -static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned int npages, size_t count, int flags)
> +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)
> {
> struct nfs_page *req;
> struct page **pages;
> @@ -321,7 +326,7 @@ static int nfs_pagein_one(struct inode *inode, struct list_head *head, unsigned
> }
> req = nfs_list_entry(data->pages.next);
>
> - return nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0);
> + return nfs_read_rpcsetup(req, data, &nfs_read_full_ops, count, 0, lseg);
> out_bad:
> nfs_async_read_error(head);
> return ret;
> @@ -625,7 +630,6 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
> if (ret == 0)
> goto read_complete; /* all pages were read */
>
> - pnfs_update_layout(inode, desc.ctx, IOMODE_READ);
> if (rsize < PAGE_CACHE_SIZE)
> nfs_pageio_init(&pgio, inode, nfs_pagein_multi, rsize, 0);
> else
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index c8278f4..004c28b 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -879,7 +879,7 @@ 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)
> +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)
> {
> struct nfs_page *req = nfs_list_entry(head->next);
> struct page *page = req->wb_page;
> @@ -946,7 +946,7 @@ 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)
> +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)
> {
> struct nfs_page *req;
> struct page **pages;
> diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
> index d55cee7..2db0372 100644
> --- a/include/linux/nfs_page.h
> +++ b/include/linux/nfs_page.h
> @@ -59,9 +59,10 @@ 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);
> + int (*pg_doio)(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *);
> int pg_ioflags;
> int pg_error;
> + struct pnfs_layout_segment *pg_lseg;
> };
>
> #define NFS_WBACK_BUSY(req) (test_bit(PG_BUSY,&(req)->wb_flags))
> @@ -79,7 +80,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),
> + int (*doio)(struct inode *, struct list_head *, unsigned int, size_t, int, struct pnfs_layout_segment *),
> size_t bsize,
> int how);
> extern int nfs_pageio_add_request(struct nfs_pageio_descriptor *,
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 51bfadb..37e91c3 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1017,6 +1017,7 @@ struct nfs_read_data {
> struct nfs_readargs args;
> struct nfs_readres res;
> unsigned long timestamp; /* For lease renewal */
> + struct pnfs_layout_segment *lseg;
> struct page *page_array[NFS_PAGEVEC_SIZE];
> };
>
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
next prev parent reply other threads:[~2011-02-14 23:14 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-14 19:18 [PATCH 0/16] pnfs wave 3 submission andros
2011-02-14 19:18 ` [PATCH 01/16] NFS remove unnecessary CONFIG_NFS_V4 from nfs_read_data andros
2011-02-15 9:16 ` Christoph Hellwig
2011-02-15 9:24 ` Taousif_Ansari-G5Y5guI6XLZWk0Htik3J/w
2011-02-15 14:51 ` Andy Adamson
2011-02-14 19:18 ` [PATCH 02/16] NFS put_layout_hdr can remove nfsi->layout andros
2011-02-14 19:18 ` [PATCH 03/16] NFS move nfs_client initialization into nfs_get_client andros
2011-02-16 2:58 ` Benny Halevy
2011-02-16 16:00 ` Andy Adamson
2011-02-14 19:18 ` [PATCH 04/16] pnfs: wave 3: send zero stateid seqid on v4.1 i/o andros
2011-02-14 19:18 ` [PATCH 05/16] pnfs: wave 3: new flag for state renewal check andros
2011-02-14 19:18 ` [PATCH 06/16] pnfs: wave 3: new flag for lease time check andros
2011-02-14 19:18 ` [PATCH 07/16] pnfs: wave 3: add MDS mount DS only check andros
2011-02-14 19:18 ` [PATCH 08/16] pnfs: wave 3: lseg refcounting andros
2011-02-15 9:25 ` Christoph Hellwig
2011-02-15 14:48 ` Fred Isaman
2011-02-15 14:58 ` Christoph Hellwig
2011-02-15 14:59 ` Benny Halevy
2011-02-15 15:06 ` Christoph Hellwig
2011-02-15 15:11 ` Fred Isaman
2011-02-15 16:02 ` Christoph Hellwig
2011-02-15 16:37 ` William A. (Andy) Adamson
2011-02-15 19:17 ` Andy Adamson
2011-02-15 19:29 ` Benny Halevy
2011-02-15 19:30 ` Andy Adamson
2011-02-15 15:07 ` Fred Isaman
2011-02-14 19:18 ` [PATCH 09/16] pnfs: wave 3: shift pnfs_update_layout locations andros
2011-02-14 23:14 ` Trond Myklebust [this message]
2011-02-15 14:41 ` Fred Isaman
2011-02-15 15:00 ` Trond Myklebust
2011-02-16 3:11 ` Benny Halevy
2011-02-14 19:18 ` [PATCH 10/16] pnfs: wave 3: coelesce across layout stripes andros
2011-02-14 23:42 ` Trond Myklebust
2011-02-15 14:43 ` William A. (Andy) Adamson
2011-02-15 15:03 ` Trond Myklebust
[not found] ` <1297782220.10103.13.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
2011-02-15 15:10 ` Andy Adamson
2011-02-14 19:18 ` [PATCH 11/16] pnfs: wave 3: generic read andros
2011-02-14 23:36 ` Trond Myklebust
2011-02-15 14:47 ` Andy Adamson
2011-02-16 3:16 ` Benny Halevy
2011-02-16 14:53 ` Andy Adamson
2011-02-16 15:09 ` Trond Myklebust
2011-02-16 15:52 ` Benny Halevy
2011-02-16 15:56 ` Andy Adamson
2011-02-16 15:57 ` Sager, Mike
2011-02-14 19:18 ` [PATCH 12/16] pnfs: wave 3: data server connection andros
2011-02-14 19:18 ` [PATCH 13/16] pnfs: wave 3: filelayout i/o helpers andros
2011-02-15 9:31 ` Christoph Hellwig
2011-02-15 15:12 ` Andy Adamson
2011-02-14 19:18 ` [PATCH 14/16] pnfs: wave 3: filelayout read andros
2011-02-14 19:18 ` [PATCH 15/16] pnfs: wave 3: filelayout async error handler andros
2011-02-14 19:18 ` [PATCH 16/16] pnfs: wave 3: turn off pNFS on ds connection failure andros
2011-02-14 22:39 ` [PATCH 0/16] pnfs wave 3 submission Trond Myklebust
2011-02-15 14:44 ` William A. (Andy) Adamson
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=1297725252.23841.45.camel@heimdal.trondhjem.org \
--to=trond.myklebust@netapp.com \
--cc=andros@citi.umich.edu \
--cc=andros@netapp.com \
--cc=bhalevy@panasas.com \
--cc=bharrosh@panasas.com \
--cc=dhildeb@us.ibm.com \
--cc=green@linuxhacker.ru \
--cc=guotao@nrchpc.ac.cn \
--cc=iisaman@citi.umich.edu \
--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.