Messages in this thread | | | Date | Thu, 26 Apr 2007 09:16:55 +0800 | From | Fengguang Wu <> | Subject | Re: [RFC][PATCH] on-demand readahead |
| |
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |