On Wed, Apr 25, 2007 at 06:08:44PM +0200, Andi Kleen wrote: > > Yeah, the on-demand readahead can avoid _all_ lookups for small in-cache > > files. > > How?
In filemap.c: if (!page) { page_cache_readahead_adaptive(mapping, &ra, filp, page, index, last_index - index); page = find_get_page(mapping, index); } if (page && PageReadahead(page)) { page_cache_readahead_adaptive(mapping, &ra, filp, page, index, last_index - index); } Cache hot files neither have missing pages (!page) or lookahead pages (PageReadahead(page)). So it will not even be called. > > > 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. > > I mean the 16 and 4 not the sysctl The numbers and the code in get_next_ra_size2() is simply copied from get_next_ra_size(): if (cur < max / 16) { newsize = 4 * cur; } else { newsize = 2 * cur; } It's a trick to ramp up small sizes more quickly. That trick is documented in the related get_init_ra_size(). So, it would be better to put the two routines together to make it clear. > > > > 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. > > If they actually help and don't cause regressions they shouldn't be a module, > but integrated eventually Just it has to be all step by step. Yeah, the adaptive readahead is complex and the possible workloads diverse. It becomes obvious that there is a long way to go, and kernel module makes life easier. > > > Your white space also needs some work. > > > > White space in patch description? > > In the code indentation. Ah, got it: a silly copy/paste mistake. Thank you, Wu - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/