Messages in this thread | | | Subject | Re: [PATCH 3/4] readahead: factor out duplicated code | From | Ram <> | Date | Wed, 02 Feb 2005 18:43:14 -0800 |
| |
On Sat, 2005-01-29 at 03:35, Oleg Nesterov wrote: > > This patch introduces make_ahead_window() function for > > simplification of page_cache_readahead. > > If you will count this patch acceptable, I'll rediff it against > next mm iteration. > > For your convenience here is the code with the patch applied. > > int make_ahead_window(mapping, filp, ra, int force) > { > int block, ret; > > ra->ahead_size = get_next_ra_size(ra); > ra->ahead_start = ra->start + ra->size; > > block = force || (ra->prev_page >= ra->ahead_start); > ret = blockable_page_cache_readahead(mapping, filp, > ra->ahead_start, ra->ahead_size, ra, block); > > if (!ret && !force) {
As steve pointed out this should be : if ( !ret && ! block ) {
> ra->ahead_start = 0; > ra->ahead_size = 0; > } > > return ret; > } > > unsigned long page_cache_readahead(mapping, ra, filp, offset, req_size) > { > unsigned long max, newsize = req_size; > int sequential = (offset == ra->prev_page + 1); > > if (offset == ra->prev_page && req_size == 1 && ra->size != 0) > goto out; > ra->prev_page = offset; > max = get_max_readahead(ra); > newsize = min(req_size, max); > > if (newsize == 0 || (ra->flags & RA_FLAG_INCACHE)) { > newsize = 1;
At this point prev_page has to be updated: ra->prev_page = offset;
> goto out; > } > > ra->prev_page += newsize - 1; > > if ((ra->size == 0 && offset == 0) || > (ra->size == -1 && sequential)) { > ra->size = get_init_ra_size(newsize, max); > ra->start = offset; > if (!blockable_page_cache_readahead(mapping, filp, offset, ra->size, ra, 1)) > goto out; > > if (req_size >= max) > make_ahead_window(mapping, filp, ra, 1); > > goto out; > } > > if (!sequential || (ra->size == 0)) { > ra_off(ra); > blockable_page_cache_readahead(mapping, filp, offset, newsize, ra, 1); > goto out; > } > > > if (ra->ahead_start == 0) { > if (!make_ahead_window(mapping, filp, ra, 0)) > goto out; > } > > if (ra->prev_page >= ra->ahead_start) { > ra->start = ra->ahead_start; > ra->size = ra->ahead_size; > make_ahead_window(mapping, filp, ra, 0); > } > out: > return newsize; > } Otherwise this code looks much cleaner and correct. Can you send me a updated patch. I will run it through my test harness.
RP
- 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/
| |