* Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> diff --git a/include/linux/futex.h b/include/linux/futex.h
> index 1d3f7555825ec..40bc778b2bb45 100644
> --- a/include/linux/futex.h
> +++ b/include/linux/futex.h
> @@ -85,7 +85,8 @@ void futex_hash_free(struct mm_struct *mm);
>  
>  static inline void futex_mm_init(struct mm_struct *mm)
>  {
> -	mm->futex_phash =  NULL;
> +	rcu_assign_pointer(mm->futex_phash, NULL);
> +	mutex_init(&mm->futex_hash_lock);
>  }

This breaks the build on older compilers - I tried gcc-9, x86-64 
defconfig:

  CC      io_uring/futex.o
 In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
                 from ./include/linux/compiler.h:390,
                 from ./include/linux/array_size.h:5,
                 from ./include/linux/kernel.h:16,
                 from io_uring/futex.c:2:
 ./include/linux/futex.h: In function 'futex_mm_init':
 ./include/linux/rcupdate.h:555:36: error: dereferencing pointer to incomplete type 'struct futex_private_hash'
  555 | #define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
      |                                    ^~~~
 ./include/asm-generic/rwonce.h:55:33: note: in definition of macro '__WRITE_ONCE'
   55 |  *(volatile typeof(x) *)&(x) = (val);    \
      |                                 ^~~
 ./arch/x86/include/asm/barrier.h:63:2: note: in expansion of macro 'WRITE_ONCE'
   63 |  WRITE_ONCE(*p, v);      \
      |  ^~~~~~~~~~
 ./include/asm-generic/barrier.h:172:55: note: in expansion of macro '__smp_store_release'
  172 | #define smp_store_release(p, v) do { kcsan_release(); __smp_store_release(p, v); } while (0)
      |                                                       ^~~~~~~~~~~~~~~~~~~
 ./include/linux/rcupdate.h:596:3: note: in expansion of macro 'smp_store_release'
  596 |   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
      |   ^~~~~~~~~~~~~~~~~
 ./include/linux/rcupdate.h:596:25: note: in expansion of macro 'RCU_INITIALIZER'
  596 |   smp_store_release(&p, RCU_INITIALIZER((typeof(p))_r_a_p__v)); \
      |                         ^~~~~~~~~~~~~~~
 ./include/linux/futex.h:91:2: note: in expansion of macro 'rcu_assign_pointer'
   91 |  rcu_assign_pointer(mm->futex_phash, NULL);
      |  ^~~~~~~~~~~~~~~~~~
 make[3]: *** [scripts/Makefile.build:203: io_uring/futex.o] Error 1
 make[2]: *** [scripts/Makefile.build:461: io_uring] Error 2
 make[1]: *** [/home/mingo/tip/Makefile:2004: .] Error 2
 make: *** [Makefile:248: __sub-make] Error 2

The problem appears to be that this variant of rcu_assign_pointer() 
wants to know the full type of 'struct futex_private_hash', which type 
is local to futex.c:

   kernel/futex/core.c:struct futex_private_hash {

So either we uninline futex_mm_init() and move it into futex/core.c, or 
we share the structure definition with kernel/fork.c. Both have 
disadvantages.

A third solution would be to just initialize mm->futex_phash with NULL 
like the patch below, it's not like this new MM's ->futex_phash can be 
observed externally until the task is inserted into the task list - 
which guarantees full store ordering.

This relaxation of this initialization might also give a tiny speedup 
on certain platforms.

But an Ack from PeterZ on that assumption would be nice.

Thanks,

	Ingo

=====================================>
Signed-off-by: Ingo Molnar <mingo@kernel.org>

 include/linux/futex.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index eccc99751bd9..168ffd5996b4 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -88,7 +88,14 @@ void futex_hash_free(struct mm_struct *mm);
 
 static inline void futex_mm_init(struct mm_struct *mm)
 {
-	rcu_assign_pointer(mm->futex_phash, NULL);
+	/*
+	 * No need for rcu_assign_pointer() here, as we can rely on
+	 * tasklist_lock write-ordering in copy_process(), before
+	 * the task's MM becomes visible and the ->futex_phash
+	 * becomes externally observable:
+	 */
+	mm->futex_phash = NULL;
+
 	mutex_init(&mm->futex_hash_lock);
 }

The following commit has been merged into the locking/futex branch of tip:

Commit-ID:     094ac8cff7858bee5fa4554f6ea66c964f8e160e
Gitweb:        https://git.kernel.org/tip/094ac8cff7858bee5fa4554f6ea66c964f8e160e
Author:        Ingo Molnar <mingo@kernel.org>
AuthorDate:    Sat, 10 May 2025 10:45:28 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sun, 11 May 2025 10:02:12 +02:00

futex: Relax the rcu_assign_pointer() assignment of mm->futex_phash in futex_mm_init()

The following commit added an rcu_assign_pointer() assignment to
futex_mm_init() in <linux/futex.h>:

  bd54df5ea7ca ("futex: Allow to resize the private local hash")

Which breaks the build on older compilers (gcc-9, x86-64 defconfig):

   CC      io_uring/futex.o
   In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
                    from ./include/linux/compiler.h:390,
                    from ./include/linux/array_size.h:5,
                    from ./include/linux/kernel.h:16,
                    from io_uring/futex.c:2:
   ./include/linux/futex.h: In function 'futex_mm_init':
   ./include/linux/rcupdate.h:555:36: error: dereferencing pointer to incomplete type 'struct futex_private_hash'

The problem is that this variant of rcu_assign_pointer() wants to
know the full type of 'struct futex_private_hash', which type
is local to futex.c:

   kernel/futex/core.c:struct futex_private_hash {

There are a couple of mechanical solutions for this bug:

  - we can uninline futex_mm_init() and move it into futex/core.c

  - or we can share the structure definition with kernel/fork.c.

But both of these solutions have disadvantages: the first one adds
runtime overhead, while the second one dis-encapsulates private
futex types.

A third solution, implemented by this patch, is to just initialize
mm->futex_phash with NULL like the patch below, it's not like this
new MM's ->futex_phash can be observed externally until the task
is inserted into the task list, which guarantees full store ordering.

The relaxation of this initialization might also give a tiny speedup
on certain platforms.

Fixes: bd54df5ea7ca ("futex: Allow to resize the private local hash")
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: André Almeida <andrealmeid@igalia.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Waiman Long <longman@redhat.com>
Link: https://lore.kernel.org/r/aB8SI00EHBri23lB@gmail.com
---
 include/linux/futex.h |  9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index eccc997..168ffd5 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -88,7 +88,14 @@ void futex_hash_free(struct mm_struct *mm);
 
 static inline void futex_mm_init(struct mm_struct *mm)
 {
-	rcu_assign_pointer(mm->futex_phash, NULL);
+	/*
+	 * No need for rcu_assign_pointer() here, as we can rely on
+	 * tasklist_lock write-ordering in copy_process(), before
+	 * the task's MM becomes visible and the ->futex_phash
+	 * becomes externally observable:
+	 */
+	mm->futex_phash = NULL;
+
 	mutex_init(&mm->futex_hash_lock);
 }