Re: [v1 PATCH 9/10] rhashtable: Allow GFP_ATOMIC bucket table allocation

On Tue, 2015-03-24 at 14:09 +1100, Herbert Xu wrote:

On Mon, Mar 23, 2015 at 12:53:11PM +0000, David Laight wrote:

quoted

Given the lock is only used for insert and delete, I'm also not at
all clear why you allocate 128 locks per cpu for very large tables.
With the locks in their own array I don't think there can be 'false
sharing', the worst than can happen is two cpus spinning on locks
in the same cache line.

Personally I'm totally against Bucket locks.  If you have a
scalability problem you really need to solve them at a higher
level, e.g., multiqueue transmission in networking.  Bucket
locks are simply kicking the can down the road, it'll come back
to bite you sooner or later in terms of scalability.

Well, keep in mind a lock can be very big with LOCKDEP.

One lock per bucket is totally overkill.

A hash array of locks is a good compromise.

128 locks per cpu is also a good compromise, because one cache line can
hold 16 locks.

Number of locks has nothing to do with number of buckets,
unless you have unlimited memory and can afford one lock per bucket,
and don't care of memory thrashing when dumping whole hash table.

I thought this kind of solution was well understood among network
developers.