Re: [PATCH] INET : removes per bucket rwlock in tcp/dccp ehash table

[Andi Kleen <ak@suse.de>]

On Thursday 01 November 2007 11:16:20 Eric Dumazet wrote:

Looks good from a quick look. Thanks for doing that work.

Some quick comments:

> +#if defined(CONFIG_SMP) || defined(CONFIG_PROVE_LOCKING)
> +/*
> + * Instead of using one rwlock for each inet_ehash_bucket, we use a table of locks
> + * The size of this table is a power of two and depends on the number of CPUS.
> + */

This shouldn't be hard coded based on NR_CPUS, but be done on runtime
based on num_possible_cpus(). This is better for kernels with a large
NR_CPUS, but which typically run on much smaller systems (like 
distribution kernels) 

Also the EHASH_LOCK_SZ == 0 special case is a little strange. Why did
you add that?

And as a unrelated node have you tried converting the rwlocks 
into normal spinlocks? spinlocks should be somewhat cheaper
because they have less cache protocol overhead and with
the huge thash tables in Linux the chain walks should be short
anyways so not doing this in parallel is probably not a big issue.
At some point I also had a crazy idea of using a special locking
scheme that special cases the common case that a hash chain
has only one member and doesn't take a look for that at all. 

-Andi