[nfsv4] [nfsv4] Performance drop in iozone for reading large files

[Bryce Harrington]

On Fri, Aug 25, 2006 at 10:56:24PM -0400, Trond Myklebust wrote:
> On Fri, 2006-08-25 at 15:57 -0700, Bryce Harrington wrote:
> > On Fri, Aug 25, 2006 at 05:23:37PM -0400, Trond Myklebust wrote:
> > > On Fri, 2006-08-25 at 14:01 -0700, Bryce Harrington wrote:
> > > > There is a performance issue in READ operations we've noticed in iozone
> > > > runs for a while now.  If you look at this plot, of NFSv4 with 1G mem:
> > > > 
> > > >    http://crucible.osdl.org/runs/1604/test_output/iozone.sys.log.png
> > > > 
> > > > You can see that we're getting fairly steady performance up until the
> > > > 1048576kb case, where read performance drops to about a third.
> > > 
> > > Is the readahead algorithm working correctly? Could you please try
> > > applying the following patch, and then check your syslog for warnings?
> > 
> > http://crucible.osdl.org/runs/1617/nfs03.console.log
> 
> OK. Could you try the attached patch? It ought to fix that particular
> issue. I'm curious to see if it will also fix the performance problem.
> 
> Cheers,
>   Trond

Here are the results when run with this patch, :

NFSv4 w/ 256M RAM:
  before patch:
    http://crucible.osdl.org/runs/1614/test_output/iozone.sys.log.png
  after patch:
    http://crucible.osdl.org/runs/1619/test_output/iozone.sys.log.png

It doesn't appear to have had an effect on the read performance.

Bryce


> Date: Fri, 25 Aug 2006 22:56:24 -0400
> From: Trond Myklebust <Trond.Myklebust at netapp.com>
> Subject: No Subject
> 
> The problem is that we may be caching writes that would extend the file and
> create a hole in the region that we are reading. In this case, we need to
> detect the eof from the server, ensure that we zero out the pages that
> are part of the hole and mark them as up to date.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust at netapp.com>
> ---
> 
>  fs/nfs/read.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 65c0c5b..da9cf11 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -116,10 +116,17 @@ static void nfs_readpage_truncate_uninit
>  	pages = &data->args.pages[base >> PAGE_CACHE_SHIFT];
>  	base &= ~PAGE_CACHE_MASK;
>  	pglen = PAGE_CACHE_SIZE - base;
> -	if (pglen < remainder)
> +	for (;;) {
> +		if (remainder <= pglen) {
> +			memclear_highpage_flush(*pages, base, remainder);
> +			break;
> +		}
>  		memclear_highpage_flush(*pages, base, pglen);
> -	else
> -		memclear_highpage_flush(*pages, base, remainder);
> +		pages++;
> +		remainder -= pglen;
> +		pglen = PAGE_CACHE_SIZE;
> +		base = 0;
> +	}
>  }
>  
>  /*
> @@ -476,6 +483,8 @@ static void nfs_readpage_set_pages_uptod
>  	unsigned int base = data->args.pgbase;
>  	struct page **pages;
>  
> +	if (data->res.eof)
> +		count = data->args.count;
>  	if (unlikely(count == 0))
>  		return;
>  	pages = &data->args.pages[base >> PAGE_CACHE_SHIFT];
> @@ -483,11 +492,7 @@ static void nfs_readpage_set_pages_uptod
>  	count += base;
>  	for (;count >= PAGE_CACHE_SIZE; count -= PAGE_CACHE_SIZE, pages++)
>  		SetPageUptodate(*pages);
> -	/*
> -	 * Was this an eof or a short read? If the latter, don't mark the page
> -	 * as uptodate yet.
> -	 */
> -	if (count > 0 && (data->res.eof || data->args.count == data->res.count))
> +	if (count != 0)
>  		SetPageUptodate(*pages);
>  }
>  
> @@ -502,6 +507,8 @@ static void nfs_readpage_set_pages_error
>  	count += base;
>  	for (;count >= PAGE_CACHE_SIZE; count -= PAGE_CACHE_SIZE, pages++)
>  		SetPageError(*pages);
> +	if (count != 0)
> +		SetPageError(*pages);
>  }
>  
>  /*