[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aB8SI00EHBri23lB@gmail.com>
Date: Sat, 10 May 2025 10:45:23 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org,
André Almeida <andrealmeid@...lia.com>,
Darren Hart <dvhart@...radead.org>,
Davidlohr Bueso <dave@...olabs.net>, Ingo Molnar <mingo@...hat.com>,
Juri Lelli <juri.lelli@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Valentin Schneider <vschneid@...hat.com>,
Waiman Long <longman@...hat.com>
Subject: [PATCH] futex: Fix futex_mm_init() build failure on older compilers,
remove rcu_assign_pointer()
* Sebastian Andrzej Siewior <bigeasy@...utronix.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@...nel.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);
}
Powered by blists - more mailing lists