Re: [RFC][PATCH] on-demand readahead

[Fengguang Wu <fengguang.wu@gmail.com>]

On Wed, Apr 25, 2007 at 04:37:41PM +0200, Andi Kleen wrote:
> Fengguang Wu <fengguang.wu@gmail.com> writes:
> 
> > OVERHEADS
> > 
> > The new code reduced the overheads of
> > 
> > 	- excessively calling the readahead routine on small sized reads
> > 	  (the current readahead code insists on seeing all requests)
> > 
> > 	- doing a lot of pointless page-cache lookups for small cached files
> > 	  (the current readahead only turns itself off after 256 cache hits,
> > 	  unfortunately most files are < 1MB, so never see that chance)
> 
> Would it make sense to keep track in the AS if the file is completely in cache?
> Then you could probably avoid a lot of these lookups for small in cache files

Yeah, the on-demand readahead can avoid _all_ lookups for small in-cache files.
But what do you mean by AS?

> > --- linux-2.6.21-rc7-mm1.orig/mm/readahead.c
> > +++ linux-2.6.21-rc7-mm1/mm/readahead.c
> > @@ -733,6 +733,11 @@ unsigned long max_sane_readahead(unsigne
> 
> Quite simple patch, why is it that much simpler than your earlier patchkits?
> Or is that on top of them?

The earlier ones focus on features, while this one aims to be simple.
Even simpler than the current readahead, while keeping the same feature set.

The on-demand readahead is now on top of them, but can/will be made
independent.

> You seem to have a lot of magic numbers. They probably all need symbols and 
> explanations.

The magic numbers are for easier testings, and will be removed in
future.  For now, they enables convenient comparing of the two
algorithms in one kernel.

If this new algorithm has been further tested and approved, I'll
re-submit the patch in a cleaner, standalone form. The adaptive
readahead patches can be dropped then. They may better be reworked as
a kernel module.

> Your white space also needs some work.

White space in patch description?
OK, thanks.

Wu