Re: 2.2.8_andrea1.bz2

Chuck Lever (cel@monkey.org)
Thu, 13 May 1999 11:15:16 -0400 (EDT)

On Thu, 13 May 1999, Andrea Arcangeli wrote:
> Yes, agreed and I just fixed this some time ago. Now I never move pages to
> the top of the lru from getblk or find_page, but I only set the reference
> bit as the stock kernel does (nothing more). Then I shrink_mmap time I
> clear the reference bit and I put the page to the top of the list (in
> shrink_mmap I would have such cost even if there wouldn't be the reference
> bit set). This removed the bottleneck completly according to numbers
> people sent to me. The nice thing is that usually the pages at the end of
> the lru are just the old ones (reference bit not set), and with the lru I
> avoid completly to waste tons of time browsing the pagemap by hand. And
> this is an huge improvement (at least when the memory gets used and there
> are more used pages than pages that sleep in the cache only for caching
> purposes).

hadn't seen a recent version of your patch... but that sounds like a good
design.

> >> BTW, I seen the buffer.c changes of 2.2.8. You have killed (not fixed ;)
> >> flushtime. At least you could have removed also flushtime from the struct
> >> buffer_head to avoid wasting time in useless initializations ;).
> >
> >especially the logic in getblk() right after the get_hash_table() call.
> >eliminating that takes out a conditional branch that is taken only rarely,
> >but is executed every time get_hash_table() finds a matching buffer
> >(which is about 80% of the time).
>
> I didn't understood exactly what you mean here. There are no changes in
> 2.2.8 realted to getblk().

correct, there are no changes in getblk() 2.2.7 -> 2.2.8. however, if
flushtime is no longer relevant, then this piece of code is now
unnecessary:

bh = get_hash_table(dev, block, size);
if (bh) {
- if (!buffer_dirty(bh)) {
- bh->b_flushtime = 0;
- }
return bh;
}

in fact, under most circumstances, this code is exercised relatively
rarely, compared to the rate that getblk() is called. so this might be
even cooler:

bh = find_buffer(dev, block, size);
if (bh) {
bh->b_count++;
return bh;
}

but i don't know what that might do to maintainability; there is a cryptic
comment next to get_hash_table() about avoiding race conditions, so
somehow this bit of code may need to change if get_hash_table() ever
changes.

> BTW, do you ever tried to benchmark one of my latest patches
> ftp://e-mind.com/pub/andrea/kernel/2.2.8_andrea1.bz2 ? I would be courious
> to see if it make relevant differences or not.

i've been waiting for some of the recent flurry to die down, but it seems
that there's still a change in 2.2.8 that is throttling performance. when
that is resolved, i will try another benchmark.

- Chuck Lever

--
corporate:	<chuckl@netscape.com>
personal:	<chucklever@netscape.net> or <cel@monkey.org>


The Linux Scalability project:
	http://www.citi.umich.edu/projects/linux-scalability/






-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/