Re: Random file I/O regressions in 2.6

[Andrew Morton <akpm@osdl.org>]

Ram Pai <linuxram@us.ibm.com> wrote:
>
> Looks like you are right on all counts!

It's a probabilistic thing.

> I did some modifications to your
> patch and did a preliminary run with my user-level simulator. With these
> changes I am able to get rid of that extra page. Also code looks much
> simpler and adapts well to sequential and random patterns.

That is good news.

> However I have to run this under some benchmarks and see how it fares.
> Its a pre-alpha level patch.

It is nicer, thanks.  I'll add it to -mm and hopefully Meredith and co will
include it in regular performance testing.

> Can you take a quick look at the changes and see if you like it? I am
> sure you won't consider these changes a hack ;)

Couple of minor things:

> -	unsigned long preoffset=0;

yay!

> +	unsigned long average=0;

Please add spaces around '='.  But I don't think this needs to be
initialised at all.


>  	/*
>  	 * Here we detect the case where the application is performing
> @@ -394,10 +394,17 @@ page_cache_readahead(struct address_spac
>  		if (ra->serial_cnt <= (max * 2))
>  			ra->serial_cnt++;
>  	} else {
> -		ra->average = (ra->average + ra->serial_cnt) / 2;
> +		/* to avoid rounding errors, ensure that 'average' 
> +		 * tends towards the value of ra->serial_cnt.
> +		 */

multiline comment layout:

		/*
		 * To avoid rounding errors, ensure that 'average' tends
		 * towards the value of ra->serial_cnt.
		 */

(I said "minor").

I can't say that I immediately understand what is the issue here with
rounding errors?


> +                if(ra->average > ra->serial_cnt) {

space between "if" and "("

> +			ra->next_size = (ra->average > max ?  
> +				max : ra->average); 

	min(max, ra->average) ?