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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/