linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
To: Andrea Arcangeli <andrea@suse.de>
Cc: Chuck Lever <cel@citi.umich.edu>,
	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 13:51:33 -0200 (BRST)	[thread overview]
Message-ID: <Pine.LNX.4.44.0312051140220.1782-100000@logos.cnet> (raw)
In-Reply-To: <20031205013205.GA1659@dualathlon.random>



On Fri, 5 Dec 2003, Andrea Arcangeli wrote:

> 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.

Patch applied. 




  reply	other threads:[~2003-12-05 16:33 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
2003-12-05 15:51   ` Marcelo Tosatti [this message]
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=Pine.LNX.4.44.0312051140220.1782-100000@logos.cnet \
    --to=marcelo.tosatti@cyclades.com \
    --cc=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).