linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@suse.de>
To: Chuck Lever <cel@citi.umich.edu>
Cc: Marcelo Tosatti <marcelo@conectiva.com.br>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] 2.4 read ahead never reads the last page in a file
Date: Fri, 5 Dec 2003 02:32:05 +0100	[thread overview]
Message-ID: <20031205013205.GA1659@dualathlon.random> (raw)
In-Reply-To: <Pine.BSO.4.33.0312031800270.24127-100000@citi.umich.edu>

On Wed, Dec 03, 2003 at 06:10:31PM -0500, Chuck Lever wrote:
> hi marcelo-
> 
> i posted this a while back on fsdevel for comments, but never heard
> anything.  i'd like this patch to be included in 2.4.24, as it improves
> NFS client read performance by a significant margin, and doesn't appear to
> have any negative impact (see fsdevel archives for benchmarks).  this is
> against 2.4.23.
> 
> generic_file_readahead never reads the last page of a file.  this means
> the last page is always read synchronously by do_generic_file_read.
> normally this is not an issue, as most local disk reads are fast.

sending two sync req is quite slower than 1 sync req even with the disk,
infact in my experience the biggest issue with fast storage isn't the
seeking but the need of sending big requests down to the hardware.
Infact I think it's a _much_ bigger issue for disk than for nfs. Can you
benchmark it on a IDE with 128k long files? They should be read with a
single command, two commands are going to hurt.

> diff -X /home/cel/src/linux/dont-diff -Naurp 00-stock/mm/filemap.c 01-readahead1/mm/filemap.c
> --- 00-stock/mm/filemap.c	2003-11-28 13:26:21.000000000 -0500
> +++ 01-readahead1/mm/filemap.c	2003-12-03 16:46:11.000000000 -0500
> @@ -1299,11 +1299,14 @@ static void generic_file_readahead(int r
>   */
>  	ahead = 0;
>  	while (ahead < max_ahead) {
> -		ahead ++;
> -		if ((raend + ahead) >= end_index)
> +		unsigned long ra_index = raend + ahead + 1;
> +
> +		if (ra_index > end_index)
>  			break;
> -		if (page_cache_read(filp, raend + ahead) < 0)
> +		if (page_cache_read(filp, ra_index) < 0)
>  			break;
> +
> +		ahead++;
>  	}
>  /*
>   * If we tried to read ahead some pages,
> 

looks ok to me. I guess this could be an historic paranoid/(possibly
needed in the past) thing, to avoid reading partial pages from there,
but the pagecache layer is robust enough to handle it these days.

Thanks.

  reply	other threads:[~2003-12-05  1:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-12-03 23:10 [PATCH] 2.4 read ahead never reads the last page in a file Chuck Lever
2003-12-05  1:32 ` Andrea Arcangeli [this message]
2003-12-05 15:51   ` Marcelo Tosatti
2003-12-05  1:54 ` Mike Fedyk
2003-12-06  8:20 ` Willy Tarreau

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=20031205013205.GA1659@dualathlon.random \
    --to=andrea@suse.de \
    --cc=cel@citi.umich.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcelo@conectiva.com.br \
    /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).