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

Reply via email to