Re: Fix knfsd readahead cache in 2.4.15

Neil Brown (neilb@cse.unsw.edu.au)
Tue, 27 Nov 2001 11:29:47 +1100 (EST)


On Monday November 26, davem@redhat.com wrote:
> From: Neil Brown <neilb@cse.unsw.edu.au>
> Date: Tue, 27 Nov 2001 10:35:26 +1100 (EST)
>
> This is definately a bug, but I don't think it is quite as dramatic as
> you suggest.
>
> The "struct raparms" that ra points to will almost always be the last
> one on the list, so ra->p_next will almost always be NULL anyway.
> Nevertheless, it is a bug and your fix looks good.
>
> There are other problems remaining, this function is a logical
> mess.
>
> 1) depth is computed in the loop, but thrown away.
> It is basically reassigned to a constant before
> being used to index the statistics it is for.

Only if the match is not found. If it is found, then we "goto" over
the re-assignment and use the counted value.

>
> 2) raparm_cache is reassigned, and since the ra param list is
> NULL terminated this can make the list shorter and shorter
> and shorter until it is one entry deep.

But it doesn't.
If the entry that was found is not at the head of the list, we delete
it from the list, and then insert it at the head of the list. Nothing
gets lost.

Given that the stats on my machine still show that it is sometimes
finding entries in the last 10% of the 64 entry cache, I am quite
confident that there are atleast 58 entryies still in the cache.
Nothing has been lost.

I don't think your patch adds anything.
Re-writing the code to use list.h lists would probably be useful
But currently (except of the problem Trond found) the code is correct.

NeilBrown

>
> Yikes :)
-
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/