Re: Random file I/O regressions in 2.6

Andrew Morton
2004-05-11T21:14:57Z

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) ?