linux-kernel - Re: futex performance regression from "futex: Allow automatic allocation of process wide futex hash"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <079dd4850554107056c536cbcb321572deaa8fde.camel@linux.intel.com>
Date: Fri, 27 Jun 2025 15:48:12 -0700
From: Tim Chen <tim.c.chen@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>, Sebastian Andrzej Siewior
	 <bigeasy@...utronix.de>
Cc: Chris Mason <clm@...a.com>, linux-kernel@...r.kernel.org, Thomas
 Gleixner <tglx@...utronix.de>
Subject: Re: futex performance regression from "futex: Allow automatic
 allocation of process wide futex hash"

On Tue, 2025-06-24 at 21:01 +0200, Peter Zijlstra wrote:
> 
> How about something like this (very lightly tested)...
> 
> the TL;DR is that it turns all those refcounts into per-cpu ops when
> there is no hash replacement pending (eg. the normal case), and only
> folds the lot into an atomic when we really care about it.
> 
> There's some sharp corners still.. but it boots and survives the
> (slightly modified) selftest.
> 
> ---
>  include/linux/futex.h                              |  12 +-
>  include/linux/mm_types.h                           |   5 +
>  kernel/futex/core.c                                | 238 +++++++++++++++++++--
>  .../selftests/futex/functional/futex_priv_hash.c   |  11 +-
>  4 files changed, 239 insertions(+), 27 deletions(-)
> 
> diff --git a/include/linux/futex.h b/include/linux/futex.h
> index b37193653e6b..5f92c7a8ba57 100644
> --- a/include/linux/futex.h
> +++ b/include/linux/futex.h
> @@ -85,17 +85,11 @@ int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4)
>  #ifdef CONFIG_FUTEX_PRIVATE_HASH
>  int futex_hash_allocate_default(void);
>  void futex_hash_free(struct mm_struct *mm);
> -
> -static inline void futex_mm_init(struct mm_struct *mm)
> -{
> -	RCU_INIT_POINTER(mm->futex_phash, NULL);
> -	mm->futex_phash_new = NULL;
> -	mutex_init(&mm->futex_hash_lock);
> -}
> +int futex_mm_init(struct mm_struct *mm);
>  
>  #else /* !CONFIG_FUTEX_PRIVATE_HASH */
>  static inline int futex_hash_allocate_default(void) { return 0; }
> -static inline void futex_hash_free(struct mm_struct *mm) { }
> +static inline int futex_hash_free(struct mm_struct *mm) { return 0; }

Minor nit.

futex_hash_free() is defined to return void for CONFIG_FUTEX_PRIVATE_HASH
config. But is now defined to return int for !CONFIG_FUTEX_PRIVATE_HASH and !CONFIG_FUTEX configs.
But it seems like nobody is actually checking the return value.  It
seems to make more sense to keep the return value as void here so
the return value is consistent?

Tim

>  static inline void futex_mm_init(struct mm_struct *mm) { }
>  #endif /* CONFIG_FUTEX_PRIVATE_HASH */
>  
> @@ -118,7 +112,7 @@ static inline int futex_hash_allocate_default(void)
>  {
>  	return 0;
>  }
> -static inline void futex_hash_free(struct mm_struct *mm) { }
> +static inline int futex_hash_free(struct mm_struct *mm) { return 0; }
>  static inline void futex_mm_init(struct mm_struct *mm) { }
>  
>  #endif