Re: [RFC patch 0/5] futex: Allow lockless empty check of hashbucket plist in futex_wake()

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Nov 27, 2013 at 11:44:38PM -0800, Davidlohr Bueso wrote:
> How about both enlarging the table _and_ aligning the buckets? As you
> know, increasing the size of the table also benefits (particularly in
> larger systems) in having more spinlocks. So we reduce the amount of
> collisions and alleviate contention on the hb->lock. Btw, do you have
> any particular concerns about the larger hash table patch? 

My only concern was the amount of #ifdef.

Wouldn't something like the below also work?


---
 kernel/futex.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 80ba086f021d..2e1a9294f751 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -70,8 +70,6 @@
 
 int __read_mostly futex_cmpxchg_enabled;
 
-#define FUTEX_HASHBITS (CONFIG_BASE_SMALL ? 4 : 8)
-
 /*
  * Futex flags used to encode options to functions and preserve them across
  * restarts.
@@ -149,9 +147,11 @@ static const struct futex_q futex_q_init = {
 struct futex_hash_bucket {
 	spinlock_t lock;
 	struct plist_head chain;
-};
+} ____cacheline_aligned_in_smp;
 
-static struct futex_hash_bucket futex_queues[1<<FUTEX_HASHBITS];
+static unsigned long __read_mostly futex_hashsize;
+
+static struct futex_hash_bucket *futex_queues;
 
 /*
  * We hash on the keys returned from get_futex_key (see below).
@@ -161,7 +161,7 @@ static struct futex_hash_bucket *hash_futex(union futex_key *key)
 	u32 hash = jhash2((u32*)&key->both.word,
 			  (sizeof(key->both.word)+sizeof(key->both.ptr))/4,
 			  key->both.offset);
-	return &futex_queues[hash & ((1 << FUTEX_HASHBITS)-1)];
+	return &futex_queues[hash & (futex_hashsize - 1)];
 }
 
 /*
@@ -2735,6 +2735,16 @@ static int __init futex_init(void)
 	u32 curval;
 	int i;
 
+#ifdef CONFIG_BASE_SMALL
+	futex_hashsize = 16;
+#else
+	futex_hashsize = roundup_pow_of_two(256 * num_possible_cpus());
+#endif
+
+	futex_queues = alloc_large_system_hash("futex", sizeof(*futex_queues),
+			futex_hashsize, 0, futex_hashsize < 256 ? HASH_SMALL : 0,
+			NULL, NULL, futex_hashsize, futex_hashsize);
+
 	/*
 	 * This will fail and we want it. Some arch implementations do
 	 * runtime detection of the futex_atomic_cmpxchg_inatomic()