From: Boaz Harrosh <bharrosh@panasas.com>
To: Benny Halevy <bhalevy@panasas.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH v8 28/32] NFSv4.1: unify pnfs_pageio_init functions
Date: Fri, 27 May 2011 18:28:10 +0300 [thread overview]
Message-ID: <4DDFC30A.6060606@panasas.com> (raw)
In-Reply-To: <1306358998-17872-1-git-send-email-bhalevy@panasas.com>
On 05/26/2011 12:29 AM, Benny Halevy wrote:
> Use common code for pnfs_pageio_init_{read,write} and use
> a common generic pg_test function.
>
> Note that this function always assumes the the layout driver's
> pg_test method is implemented.
>
murphy law for programming says that if you do not test some code
it will fail no matter how simple it is.
Even code that if you'd test it it would be fine. The fact you did
not test it is the cause of why it fails. (You let a low chance
possibility in)
So with this patch pnfs stops working!
I'll send a patch as reply
> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
> ---
> fs/nfs/pagelist.c | 1 +
> fs/nfs/pnfs.c | 57 ++++++++++++++--------------------------------------
> fs/nfs/pnfs.h | 19 +++++++----------
> fs/nfs/read.c | 2 +-
> fs/nfs/write.c | 3 +-
> 5 files changed, 27 insertions(+), 55 deletions(-)
>
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index c80add6..0918ea8 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -229,6 +229,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
> desc->pg_ioflags = io_flags;
> desc->pg_error = 0;
> desc->pg_lseg = NULL;
> + desc->pg_test = NULL;
> }
>
> /**
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 00b1282..f7a9405 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1043,46 +1043,30 @@ out_forget_reply:
> goto out;
> }
>
> -static int pnfs_read_pg_test(struct nfs_pageio_descriptor *pgio,
> - struct nfs_page *prev,
> - struct nfs_page *req)
> -{
> - if (pgio->pg_count == prev->wb_bytes) {
> - /* This is first coelesce call for a series of nfs_pages */
> - pgio->pg_lseg = pnfs_update_layout(pgio->pg_inode,
> - prev->wb_context,
> - req_offset(req),
> - pgio->pg_count,
> - IOMODE_READ,
> - GFP_KERNEL);
> - } else if (pgio->pg_lseg &&
> - req_offset(req) > end_offset(pgio->pg_lseg->pls_range.offset,
> - pgio->pg_lseg->pls_range.length))
> - return 0;
> - return NFS_SERVER(pgio->pg_inode)->pnfs_curr_ld->pg_test(pgio, prev, req);
> -}
> -
> -void
> -pnfs_pageio_init_read(struct nfs_pageio_descriptor *pgio, struct inode *inode)
> +int
> +pnfs_generic_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
> + struct nfs_page *req)
> {
> - struct pnfs_layoutdriver_type *ld;
> + enum pnfs_iomode access_type;
> + gfp_t gfp_flags;
>
> - ld = NFS_SERVER(inode)->pnfs_curr_ld;
> - pgio->pg_test = (ld && ld->pg_test) ? pnfs_read_pg_test : NULL;
> -}
> -
> -static int pnfs_write_pg_test(struct nfs_pageio_descriptor *pgio,
> - struct nfs_page *prev,
> - struct nfs_page *req)
> -{
> + /* We assume that pg_ioflags == 0 iff we're reading a page */
> + if (pgio->pg_ioflags == 0) {
> + access_type = IOMODE_READ;
> + gfp_flags = GFP_KERNEL;
> + } else {
> + access_type = IOMODE_RW;
> + gfp_flags = GFP_NOFS;
> + }
> +
nit, Two extra tabs
Boaz
> if (pgio->pg_count == prev->wb_bytes) {
> /* This is first coelesce call for a series of nfs_pages */
> pgio->pg_lseg = pnfs_update_layout(pgio->pg_inode,
> prev->wb_context,
> req_offset(req),
> pgio->pg_count,
> - IOMODE_RW,
> - GFP_NOFS);
> + access_type,
> + gfp_flags);
> } else if (pgio->pg_lseg &&
> req_offset(req) > end_offset(pgio->pg_lseg->pls_range.offset,
> pgio->pg_lseg->pls_range.length))
> @@ -1090,15 +1074,6 @@ static int pnfs_write_pg_test(struct nfs_pageio_descriptor *pgio,
> return NFS_SERVER(pgio->pg_inode)->pnfs_curr_ld->pg_test(pgio, prev, req);
> }
>
> -void
> -pnfs_pageio_init_write(struct nfs_pageio_descriptor *pgio, struct inode *inode)
> -{
> - struct pnfs_layoutdriver_type *ld;
> -
> - ld = NFS_SERVER(inode)->pnfs_curr_ld;
> - pgio->pg_test = (ld && ld->pg_test) ? pnfs_write_pg_test : NULL;
> -}
> -
> /*
> * Called by non rpc-based layout drivers
> */
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 25da946..cc61ae0 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -158,8 +158,7 @@ enum pnfs_try_status pnfs_try_to_write_data(struct nfs_write_data *,
> const struct rpc_call_ops *, int);
> enum pnfs_try_status pnfs_try_to_read_data(struct nfs_read_data *,
> const struct rpc_call_ops *);
> -void pnfs_pageio_init_read(struct nfs_pageio_descriptor *, struct inode *);
> -void pnfs_pageio_init_write(struct nfs_pageio_descriptor *, struct inode *);
> +int pnfs_generic_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev, struct nfs_page *req);
> int pnfs_layout_process(struct nfs4_layoutget *lgp);
> void pnfs_free_lseg_list(struct list_head *tmp_list);
> void pnfs_destroy_layout(struct nfs_inode *);
> @@ -293,6 +292,12 @@ 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)
> +{
> + if (NFS_SERVER(inode)->pnfs_curr_ld)
> + pgio->pg_test = pnfs_generic_pg_test;
> +}
> +
> #else /* CONFIG_NFS_V4_1 */
>
> static inline void pnfs_destroy_all_layouts(struct nfs_client *clp)
> @@ -376,16 +381,8 @@ static inline void unset_pnfs_layoutdriver(struct nfs_server *s)
> {
> }
>
> -static inline void
> -pnfs_pageio_init_read(struct nfs_pageio_descriptor *pgio, struct inode *ino)
> -{
> - pgio->pg_test = NULL;
> -}
> -
> -static inline void
> -pnfs_pageio_init_write(struct nfs_pageio_descriptor *pgio, struct inode *ino)
> +static inline void pnfs_pageio_init(struct nfs_pageio_descriptor *, struct inode *)
> {
> - pgio->pg_test = NULL;
> }
>
> static inline void
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 540c8bc..6bd09a8 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -664,7 +664,7 @@ int nfs_readpages(struct file *filp, struct address_space *mapping,
> if (ret == 0)
> goto read_complete; /* all pages were read */
>
> - pnfs_pageio_init_read(&pgio, inode);
> + pnfs_pageio_init(&pgio, inode);
> 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 7edb72f..d81c5c0 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -1036,8 +1036,7 @@ static void nfs_pageio_init_write(struct nfs_pageio_descriptor *pgio,
> {
> size_t wsize = NFS_SERVER(inode)->wsize;
>
> - pnfs_pageio_init_write(pgio, inode);
> -
> + pnfs_pageio_init(pgio, inode);
> if (wsize < PAGE_CACHE_SIZE)
> nfs_pageio_init(pgio, inode, nfs_flush_multi, wsize, ioflags);
> else
next prev parent reply other threads:[~2011-05-27 15:28 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-25 21:24 [PATCHSET v8 0/32] pnfs for 2.6.40 Benny Halevy
2011-05-25 21:26 ` [PATCH v8 01/32] NFSv4.1: fix typo in filelayout_check_layout Benny Halevy
2011-05-25 21:26 ` [PATCH v8 02/32] NFSv4.1: use struct nfs_client to qualify deviceid Benny Halevy
2011-05-25 21:26 ` [PATCH v8 03/32] pnfs: resolve header dependency in pnfs.h Benny Halevy
2011-05-25 21:26 ` [PATCH v8 04/32] NFSv4.1: make deviceid cache global Benny Halevy
2011-05-25 21:27 ` [PATCH v8 05/32] NFSv4.1: purge deviceid cache on nfs_free_client Benny Halevy
2011-05-29 8:06 ` [PATCH] SQUASHME: into NFSv4.1: purge deviceid cache - let ver < 4.1 compile Boaz Harrosh
2011-05-25 21:27 ` [PATCH v8 06/32] pnfs: CB_NOTIFY_DEVICEID Benny Halevy
2011-05-25 21:27 ` [PATCH v8 07/32] NFSv4.1: use layout driver in global device cache Benny Halevy
2011-05-25 21:27 ` [PATCH v8 08/32] SUNRPC: introduce xdr_init_decode_pages Benny Halevy
2011-05-25 21:27 ` [PATCH v8 09/32] pnfs: Use byte-range for layoutget Benny Halevy
2011-05-25 21:27 ` [PATCH v8 10/32] pnfs: align layoutget requests on page boundaries Benny Halevy
2011-05-25 21:27 ` [PATCH v8 11/32] pnfs: Use byte-range for cb_layoutrecall Benny Halevy
2011-05-25 21:27 ` [PATCH v8 12/32] pnfs: client stats Benny Halevy
2011-05-25 21:28 ` [PATCH v8 13/32] pnfs-obj: objlayoutdriver module skeleton Benny Halevy
2011-05-25 21:28 ` [PATCH v8 14/32] pnfs-obj: pnfs_osd XDR definitions Benny Halevy
2011-05-25 21:28 ` [PATCH v8 15/32] pnfs-obj: pnfs_osd XDR client implementation Benny Halevy
2011-05-25 21:28 ` [PATCH v8 16/32] pnfs-obj: decode layout, alloc/free lseg Benny Halevy
2011-05-26 19:05 ` [PATCH] SQUASHME V2: objio alloc/free lseg Bugs fixes Boaz Harrosh
2011-05-25 21:28 ` [PATCH v8 17/32] pnfs-obj: objio_osd device information retrieval and caching Benny Halevy
2011-05-26 14:04 ` Boaz Harrosh
2011-05-26 16:09 ` Benny Halevy
2011-05-26 19:07 ` [PATCH v9 16/18] " Boaz Harrosh
2011-05-26 19:09 ` Boaz Harrosh
2011-05-29 18:14 ` Benny Halevy
2011-05-25 21:28 ` [PATCH v8 18/32] pnfs: alloc and free layout_hdr layoutdriver methods Benny Halevy
2011-05-25 21:28 ` [PATCH v8 19/32] pnfs-obj: define per-inode private structure Benny Halevy
2011-05-25 21:28 ` [PATCH v8 20/32] pnfs: support for non-rpc layout drivers Benny Halevy
2011-05-25 21:29 ` [PATCH v8 21/32] pnfs-obj: osd raid engine read/write implementation Benny Halevy
2011-05-26 19:12 ` [PATCH resend3] SQUASHME: objio read/write patch: Bugs fixes Boaz Harrosh
2011-05-25 21:29 ` [PATCH v8 22/32] pnfs: layoutreturn Benny Halevy
2011-05-27 14:25 ` Boaz Harrosh
2011-05-25 21:29 ` [PATCH v8 23/32] pnfs: layoutret_on_setattr Benny Halevy
2011-05-25 21:29 ` [PATCH v8 24/32] pnfs: encode_layoutreturn Benny Halevy
2011-05-25 21:29 ` [PATCH v8 25/32] pnfs-obj: report errors and .encode_layoutreturn Implementation Benny Halevy
2011-05-26 19:14 ` Boaz Harrosh
2011-05-26 19:15 ` [PATCH v9 " Boaz Harrosh
2011-05-25 21:29 ` [PATCH v8 26/32] pnfs: encode_layoutcommit Benny Halevy
2011-05-25 21:29 ` [PATCH v8 27/32] pnfs-obj: objlayout_encode_layoutcommit implementation Benny Halevy
2011-05-25 21:29 ` [PATCH v8 28/32] NFSv4.1: unify pnfs_pageio_init functions Benny Halevy
2011-05-27 15:28 ` Boaz Harrosh [this message]
2011-05-27 15:37 ` [PATCH] SQUASHME: Fix BUG in: " Boaz Harrosh
2011-05-29 8:09 ` Boaz Harrosh
2011-05-29 8:10 ` [PATCH v2] " Boaz Harrosh
2011-05-25 21:30 ` [PATCH v8 29/32] NFSv4.1: change pg_test return type to bool Benny Halevy
2011-05-25 21:30 ` [PATCH v8 30/32] NFSv4.1: use pnfs_generic_pg_test directly by layout driver Benny Halevy
2011-05-25 21:30 ` [PATCH v8 31/32] NFSv4.1: define nfs_generic_pg_test Benny Halevy
2011-05-26 14:18 ` Boaz Harrosh
2011-05-26 18:20 ` Benny Halevy
2011-05-26 19:03 ` Boaz Harrosh
2011-05-26 19:17 ` Subject: [PATCH] SQUASHME: Move a check from nfs_pageio_do_add_request to nfs_generic_pg_test Boaz Harrosh
2011-05-26 19:58 ` Boaz Harrosh
2011-05-26 20:11 ` Trond Myklebust
2011-05-27 7:22 ` Boaz Harrosh
2011-05-25 21:30 ` [PATCH v8 32/32] pnfs-obj: pg_test check for max_io_size Benny Halevy
2011-05-26 14:57 ` [PATCHSET v8 0/32] pnfs for 2.6.40 Boaz Harrosh
2011-05-26 16:14 ` Benny Halevy
2011-05-29 8:56 ` Boaz Harrosh
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=4DDFC30A.6060606@panasas.com \
--to=bharrosh@panasas.com \
--cc=Trond.Myklebust@netapp.com \
--cc=bhalevy@panasas.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).