[PATCH v8 13/15] futex: Resize local futex hash table based on number of threads.

Sebastian Andrzej Siewior posted 15 patches 1 year ago
There is a newer version of this series
[PATCH v8 13/15] futex: Resize local futex hash table based on number of threads.
Posted by Sebastian Andrzej Siewior 1 year ago
Automatically size the local hash based on the number of threads. The
logic tries to allocate between 16 and futex_hashsize (the default for
the system wide hash bucket) and uses 4 * number-of-threads.
On CONFIG_BASE_SMALL configs the suggested size is always 2.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/futex.h | 12 ------------
 kernel/fork.c         |  4 +---
 kernel/futex/core.c   | 34 +++++++++++++++++++++++++++++++---
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index bfb38764bac7a..6469aeb76a150 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -87,13 +87,6 @@ static inline void futex_mm_init(struct mm_struct *mm)
 	mutex_init(&mm->futex_hash_lock);
 }
 
-static inline bool futex_hash_requires_allocation(void)
-{
-	if (current->mm->futex_phash)
-		return false;
-	return true;
-}
-
 #else
 static inline void futex_init_task(struct task_struct *tsk) { }
 static inline void futex_exit_recursive(struct task_struct *tsk) { }
@@ -116,11 +109,6 @@ static inline int futex_hash_allocate_default(void)
 static inline void futex_hash_free(struct mm_struct *mm) { }
 static inline void futex_mm_init(struct mm_struct *mm) { }
 
-static inline bool futex_hash_requires_allocation(void)
-{
-	return false;
-}
-
 #endif
 
 #endif
diff --git a/kernel/fork.c b/kernel/fork.c
index 824cc55d32ece..5e15e5b24f289 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2142,9 +2142,7 @@ static bool need_futex_hash_allocate_default(u64 clone_flags)
 {
 	if ((clone_flags & (CLONE_THREAD | CLONE_VM)) != (CLONE_THREAD | CLONE_VM))
 		return false;
-	if (!thread_group_empty(current))
-		return false;
-	return futex_hash_requires_allocation();
+	return true;
 }
 
 /*
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index e1bf43f7eb277..9a12dccb1c995 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -1411,8 +1411,8 @@ static int futex_hash_allocate(unsigned int hash_slots)
 		hash_slots = 16;
 	if (hash_slots < 2)
 		hash_slots = 2;
-	if (hash_slots > 131072)
-		hash_slots = 131072;
+	if (hash_slots > futex_hashsize)
+		hash_slots = futex_hashsize;
 	if (!is_power_of_2(hash_slots))
 		hash_slots = rounddown_pow_of_two(hash_slots);
 
@@ -1454,7 +1454,35 @@ static int futex_hash_allocate(unsigned int hash_slots)
 
 int futex_hash_allocate_default(void)
 {
-	return futex_hash_allocate(0);
+	unsigned int threads, buckets, current_buckets = 0;
+	struct futex_private_hash *hb_p;
+
+	if (!current->mm)
+		return 0;
+
+	scoped_guard(rcu) {
+		threads = get_nr_threads(current);
+		hb_p = rcu_dereference(current->mm->futex_phash);
+		if (hb_p)
+			current_buckets = hb_p->hash_mask + 1;
+	}
+
+	if (IS_ENABLED(CONFIG_BASE_SMALL)) {
+		buckets = 2;
+
+	} else {
+		/*
+		 * The default allocation will remain within
+		 *   16 <= threads * 4 <= global hash size
+		 */
+		buckets = roundup_pow_of_two(4 * threads);
+		buckets = max(buckets, 16);
+		buckets = min(buckets, futex_hashsize);
+	}
+	if (current_buckets >= buckets)
+		return 0;
+
+	return futex_hash_allocate(buckets);
 }
 
 static int futex_hash_get_slots(void)
-- 
2.47.2
Re: [PATCH v8 13/15] futex: Resize local futex hash table based on number of threads.
Posted by Peter Zijlstra 1 year ago
On Mon, Feb 03, 2025 at 02:59:33PM +0100, Sebastian Andrzej Siewior wrote:
> Automatically size the local hash based on the number of threads. The
> logic tries to allocate between 16 and futex_hashsize (the default for
> the system wide hash bucket) and uses 4 * number-of-threads.
> On CONFIG_BASE_SMALL configs the suggested size is always 2.

> +	scoped_guard(rcu) {
> +		threads = get_nr_threads(current);

How about something like:

		threads = min(get_nr_threads(), num_online_cpus())

?

> +		hb_p = rcu_dereference(current->mm->futex_phash);
> +		if (hb_p)
> +			current_buckets = hb_p->hash_mask + 1;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_BASE_SMALL)) {
> +		buckets = 2;
> +

Or... you just disable the local thing entirely for BASE_SMALL and have
it fall back to the global hash.

> +	} else {
> +		/*
> +		 * The default allocation will remain within
> +		 *   16 <= threads * 4 <= global hash size
> +		 */
> +		buckets = roundup_pow_of_two(4 * threads);
> +		buckets = max(buckets, 16);
> +		buckets = min(buckets, futex_hashsize);
> +	}
> +	if (current_buckets >= buckets)
> +		return 0;
> +
> +	return futex_hash_allocate(buckets);
>  }
>  
>  static int futex_hash_get_slots(void)
> -- 
> 2.47.2
>
Re: [PATCH v8 13/15] futex: Resize local futex hash table based on number of threads.
Posted by Sebastian Andrzej Siewior 1 year ago
On 2025-02-04 11:21:46 [+0100], Peter Zijlstra wrote:
> > +	if (IS_ENABLED(CONFIG_BASE_SMALL)) {
> > +		buckets = 2;
> > +
> 
> Or... you just disable the local thing entirely for BASE_SMALL and have
> it fall back to the global hash.

If we don't assign a local hash on auto resize for CONFIG_BASE_SMALL
builds then we need to also disable PR_FUTEX_HASH_SET_SLOTS. Not against
it at all, just pointing out.

The reason is that there is at least one spot (exit_pi_state_list())
where I need a stable view of the hash. I ensure this by grabbing a
reference so this pointer does not change.
If this local hash is set to NULL then we are single threaded and it can
not be assigned. If we are multi threaded then the pointer is not NULL
and PR_FUTEX_HASH_SET_SLOTS based assignment will be delayed.
But if we are multi threaded and the local hash is set to NULL then it
could be assigned at which point the whole logic breaks.

Sebastian
Re: [PATCH v8 13/15] futex: Resize local futex hash table based on number of threads.
Posted by Sebastian Andrzej Siewior 1 year ago
On 2025-02-04 11:21:46 [+0100], Peter Zijlstra wrote:
> On Mon, Feb 03, 2025 at 02:59:33PM +0100, Sebastian Andrzej Siewior wrote:
> > Automatically size the local hash based on the number of threads. The
> > logic tries to allocate between 16 and futex_hashsize (the default for
> > the system wide hash bucket) and uses 4 * number-of-threads.
> > On CONFIG_BASE_SMALL configs the suggested size is always 2.
> 
> > +	scoped_guard(rcu) {
> > +		threads = get_nr_threads(current);
> 
> How about something like:
> 
> 		threads = min(get_nr_threads(), num_online_cpus())
> 
> ?
makes sense.

> > +		hb_p = rcu_dereference(current->mm->futex_phash);
> > +		if (hb_p)
> > +			current_buckets = hb_p->hash_mask + 1;
> > +	}
> > +
> > +	if (IS_ENABLED(CONFIG_BASE_SMALL)) {
> > +		buckets = 2;
> > +
> 
> Or... you just disable the local thing entirely for BASE_SMALL and have
> it fall back to the global hash.

Okay, why not.

> > +	} else {
> > +		/*
> > +		 * The default allocation will remain within
> > +		 *   16 <= threads * 4 <= global hash size
> > +		 */
> > +		buckets = roundup_pow_of_two(4 * threads);
> > +		buckets = max(buckets, 16);
> > +		buckets = min(buckets, futex_hashsize);
> > +	}
> > +	if (current_buckets >= buckets)
> > +		return 0;
> > +
> > +	return futex_hash_allocate(buckets);
> >  }
> >  
> >  static int futex_hash_get_slots(void)

Sebastian