On Wed, 2011-06-01 at 17:51 +0300, Benny Halevy wrote:
> I think the following should work:
>
> Benny
>
> git diff --stat -p -M
> fs/nfs/nfs4filelayout.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
> index 4269088..9f1d445 100644
> --- a/fs/nfs/nfs4filelayout.c
> +++ b/fs/nfs/nfs4filelayout.c
> @@ -661,6 +661,16 @@ filelayout_pg_test(struct nfs_pageio_descriptor
> *pgio, struct nfs_page *prev,
> u64 p_stripe, r_stripe;
> u32 stripe_unit;
>
> + /*
> + * FIXME: ideally we should be able to coalesce all requests
> + * that are not block boundary aligned, but currently this
> + * is problematic for the case of bsize < PAGE_CACHE_SIZE,
> + * since nfs_flush_multi and nfs_pagein_multi assume you
> + * can have only one struct nfs_page.
> + */
> + if (desc->pg_bsize < PAGE_SIZE)
> + return 0;
> +
> if (!pnfs_generic_pg_test(pgio, prev, req))
> return 0;
So, there are several things that bother me about pnfs_generic_pg_test()
too now that I'm looking more closely at it:
1. If the intention is to coalesce 'prev' and 'req', shouldn't we
be asking for a layout with req_offset(prev) instead of
req_offset(req)?
2. If we're only requesting a layout of length pg_count, don't we
still need to test the layout length that the server actually
returned before we can allow the coalescing?
3. if (!pgio->lseg), shouldn't we be returning an error of some
sort? Right now we're returning 'true', and allowing the
coalesce to occur.
4. Furthermore, shouldn't that test guarding the
pnfs_update_layout() call rather be an 'if (pgio->pg_lseg ==
NULL)' instead of looking at the values of pg_count and
prev->wb_bytes?
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--
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
