Re: [patch] Cache align futex hash buckets

[Nick Piggin]

Ravikiran G Thirumalai wrote:
> On Tue, Feb 21, 2006 at 02:30:00PM +1100, Nick Piggin wrote:
>
> > Ravikiran G Thirumalai wrote:
> >
> > > Following change places each element of the futex_queues hashtable on a 
> > > different cacheline.  Spinlocks of adjacent hash buckets lie on the same 
> > > cacheline otherwise.
> >
> > It does not make sense to add swaths of unused memory into a hashtable for
> > this purpose, does it?
>
>
> I don't know if having two (or more) spinlocks on the same cacheline is a good 
> idea.  Right now, on a 128 B cacheline we have 10 spinlocks on the
> same cacheline here!! Things get worse if two futexes from different nodes 
> hash on to adjacent, or even nearly adjacent hash buckets.

It is no worse than having a hash collision and having to take the same lock.

(as I said, a really good solution might just use a single lock per cacheline,
but that would make hash indexing a bit more difficult.)

> > For a minimal, naive solution you just increase the size of the hash table.
> > This will (given a decent hash function) provide the same reduction in
> > cacheline contention, while also reducing collisions.
>
>
> Given a decent hash function.  I am not sure the hashing function is smart 
> enough as of now.  Hashing is not a function of nodeid, and we have some 
> instrumentation results which show hashing on NUMA is not good as yet, and
> there are collisions from other nodes onto the same hashbucket; Nearby 
> buckets have high hit rates too.

But it is decent in that it is random. The probability of an address hashing
to the same cacheline as another should be more or less the same as with your
padded scheme.

> I think some sort of NUMA friendly hashing, where futexes from same nodes
> hash onto a node local hash table, would be a decent solution here.
> As I mentioned earlier, we are working on that, and we can probably allocate
> the spinlock from nodelocal memory then and avoid this bloat.
> We are hoping to have this as a stop gap fix until we get there.

But my point still stands that it never makes sense to add empty space into
a hash table. Add hash slots instead and you (for free) get shorter hash
chains.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/