Messages in this thread | | | Date | Thu, 10 Mar 2005 20:14:27 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/2] readahead: improve sequential read detection |
| |
Ram wrote: > > On Wed, 2005-03-02 at 11:08, Oleg Nesterov wrote: > > > > out: > > - return newsize; > > + return ra->prev_page + 1; > > This change introduces one key behavioural change in > page_cache_readahead(). Instead of returning the number-of-pages > successfully read, it now returns the next-page-index which is yet to be > read. Was this essential?
The problem is that with this change: + if (offset == ra->prev_page && --req_size) + ++offset; we can't just return newsize.
Because the caller of page_cache_readahead(offset, req_size) expects that returned value is the number-of-pages successfully read from this original offset.
Consider do_generic_mapping_read() reading two pages at offset 10, with ra->prev_page == 10.
1st page, index == 10: ret_size = page_cache_readahead(10, 2); // returns 1 next_index += ret_size; // next_index == 11
2nd page, index == 11: if (index == next_index) // Yes page_cache_readahead(11, 1); // BOGUS!
It can be solved without behavioural change of course, but it will be more complex.
> At least, a comment towards this effect at the top of the function is > worth adding.
Yes, it's my fault. I should have added comment in changelog at least.
Oleg. - 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/
| |