From: Thomas Gleixner <tglx@linutronix.de>
Replace the open coded implementation with the new get/put_user_inline()
helpers. This might be replaced by a regular get/put_user(), but that needs
a proper performance evaluation.
No functional change intended
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "André Almeida" <andrealmeid@igalia.com>
---
V5: Rename again and remove the helpers
V4: Rename once moar
V3: Adapt to scope changes
V2: Convert to scoped variant
---
kernel/futex/core.c | 4 +--
kernel/futex/futex.h | 58 ++-------------------------------------------------
2 files changed, 5 insertions(+), 57 deletions(-)
---
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -581,7 +581,7 @@ int get_futex_key(u32 __user *uaddr, uns
if (flags & FLAGS_NUMA) {
u32 __user *naddr = (void *)uaddr + size / 2;
- if (futex_get_value(&node, naddr))
+ if (get_user_inline(node, naddr))
return -EFAULT;
if ((node != FUTEX_NO_NODE) &&
@@ -601,7 +601,7 @@ int get_futex_key(u32 __user *uaddr, uns
node = numa_node_id();
node_updated = true;
}
- if (node_updated && futex_put_value(node, naddr))
+ if (node_updated && put_user_inline(node, naddr))
return -EFAULT;
}
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -281,63 +281,11 @@ static inline int futex_cmpxchg_value_lo
return ret;
}
-/*
- * This does a plain atomic user space read, and the user pointer has
- * already been verified earlier by get_futex_key() to be both aligned
- * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
- *
- * We still want to avoid any speculation, and while __get_user() is
- * the traditional model for this, it's actually slower than doing
- * this manually these days.
- *
- * We could just have a per-architecture special function for it,
- * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
- * than force everybody to do that, write it out long-hand using
- * the low-level user-access infrastructure.
- *
- * This looks a bit overkill, but generally just results in a couple
- * of instructions.
- */
-static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
-{
- u32 val;
-
- if (can_do_masked_user_access())
- from = masked_user_access_begin(from);
- else if (!user_read_access_begin(from, sizeof(*from)))
- return -EFAULT;
- unsafe_get_user(val, from, Efault);
- user_read_access_end();
- *dest = val;
- return 0;
-Efault:
- user_read_access_end();
- return -EFAULT;
-}
-
-static __always_inline int futex_put_value(u32 val, u32 __user *to)
-{
- if (can_do_masked_user_access())
- to = masked_user_access_begin(to);
- else if (!user_write_access_begin(to, sizeof(*to)))
- return -EFAULT;
- unsafe_put_user(val, to, Efault);
- user_write_access_end();
- return 0;
-Efault:
- user_write_access_end();
- return -EFAULT;
-}
-
+/* Read from user memory with pagefaults disabled */
static inline int futex_get_value_locked(u32 *dest, u32 __user *from)
{
- int ret;
-
- pagefault_disable();
- ret = futex_get_value(dest, from);
- pagefault_enable();
-
- return ret;
+ guard(pagefault)();
+ return get_user_inline(*dest, from);
}
extern void __futex_unqueue(struct futex_q *q);
Le 27/10/2025 à 09:44, Thomas Gleixner a écrit :
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Replace the open coded implementation with the new get/put_user_inline()
> helpers. This might be replaced by a regular get/put_user(), but that needs
> a proper performance evaluation.
>
> No functional change intended
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "André Almeida" <andrealmeid@igalia.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> V5: Rename again and remove the helpers
> V4: Rename once moar
> V3: Adapt to scope changes
> V2: Convert to scoped variant
> ---
> kernel/futex/core.c | 4 +--
> kernel/futex/futex.h | 58 ++-------------------------------------------------
> 2 files changed, 5 insertions(+), 57 deletions(-)
> ---
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -581,7 +581,7 @@ int get_futex_key(u32 __user *uaddr, uns
> if (flags & FLAGS_NUMA) {
> u32 __user *naddr = (void *)uaddr + size / 2;
>
> - if (futex_get_value(&node, naddr))
> + if (get_user_inline(node, naddr))
> return -EFAULT;
>
> if ((node != FUTEX_NO_NODE) &&
> @@ -601,7 +601,7 @@ int get_futex_key(u32 __user *uaddr, uns
> node = numa_node_id();
> node_updated = true;
> }
> - if (node_updated && futex_put_value(node, naddr))
> + if (node_updated && put_user_inline(node, naddr))
> return -EFAULT;
> }
>
> --- a/kernel/futex/futex.h
> +++ b/kernel/futex/futex.h
> @@ -281,63 +281,11 @@ static inline int futex_cmpxchg_value_lo
> return ret;
> }
>
> -/*
> - * This does a plain atomic user space read, and the user pointer has
> - * already been verified earlier by get_futex_key() to be both aligned
> - * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
> - *
> - * We still want to avoid any speculation, and while __get_user() is
> - * the traditional model for this, it's actually slower than doing
> - * this manually these days.
> - *
> - * We could just have a per-architecture special function for it,
> - * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
> - * than force everybody to do that, write it out long-hand using
> - * the low-level user-access infrastructure.
> - *
> - * This looks a bit overkill, but generally just results in a couple
> - * of instructions.
> - */
> -static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
> -{
> - u32 val;
> -
> - if (can_do_masked_user_access())
> - from = masked_user_access_begin(from);
> - else if (!user_read_access_begin(from, sizeof(*from)))
> - return -EFAULT;
> - unsafe_get_user(val, from, Efault);
> - user_read_access_end();
> - *dest = val;
> - return 0;
> -Efault:
> - user_read_access_end();
> - return -EFAULT;
> -}
> -
> -static __always_inline int futex_put_value(u32 val, u32 __user *to)
> -{
> - if (can_do_masked_user_access())
> - to = masked_user_access_begin(to);
> - else if (!user_write_access_begin(to, sizeof(*to)))
> - return -EFAULT;
> - unsafe_put_user(val, to, Efault);
> - user_write_access_end();
> - return 0;
> -Efault:
> - user_write_access_end();
> - return -EFAULT;
> -}
> -
> +/* Read from user memory with pagefaults disabled */
> static inline int futex_get_value_locked(u32 *dest, u32 __user *from)
> {
> - int ret;
> -
> - pagefault_disable();
> - ret = futex_get_value(dest, from);
> - pagefault_enable();
> -
> - return ret;
> + guard(pagefault)();
> + return get_user_inline(*dest, from);
> }
>
> extern void __futex_unqueue(struct futex_q *q);
>
On 2025-10-27 04:44, Thomas Gleixner wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > Replace the open coded implementation with the new get/put_user_inline() > helpers. This might be replaced by a regular get/put_user(), but that needs > a proper performance evaluation. I understand that this is aiming to keep the same underlying code, but I find it surprising that the first user of the "inline" get/put user puts the burden of the proof on moving this to regular get/put_user() rather than on using the inlined version. The comment above the inline API clearly states that performance numbers are needed to justify the use of inline, not the opposite. I am concerned that this creates a precedent that may be used by future users of the inline API to use it without performance numbers justification. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
On Tue, Oct 28 2025 at 10:24, Mathieu Desnoyers wrote: > On 2025-10-27 04:44, Thomas Gleixner wrote: >> From: Thomas Gleixner <tglx@linutronix.de> >> >> Replace the open coded implementation with the new get/put_user_inline() >> helpers. This might be replaced by a regular get/put_user(), but that needs >> a proper performance evaluation. > > I understand that this is aiming to keep the same underlying code, > but I find it surprising that the first user of the "inline" get/put > user puts the burden of the proof on moving this to regular > get/put_user() rather than on using the inlined version. > > The comment above the inline API clearly states that performance > numbers are needed to justify the use of inline, not the opposite. > > I am concerned that this creates a precedent that may be used by future > users of the inline API to use it without performance numbers > justification. There was not justification for the open coded inline either and converting it to get/put must be a completely seperate change.
On Tue, 28 Oct 2025 at 08:56, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> There was not justification for the open coded inline either and
> converting it to get/put must be a completely seperate change.
Actually, there's some justification in the original: see commit
43a43faf5376 ("futex: improve user space accesses") which talks about
the original impetus for it all: avoiding the very expensive barrier
in __get_user(), and how __get_user() itself couldn't be fixed.
So then it was converted to the modern user access helpers - including
address masking - and the inlining was mostly incidental to that, but
the commit message does point out that it actually makes the address
generation a bit cleaner in addition to avoiding the function call.
But I doubt that the extra instructions are all that noticeable.
That said - this code *is* in a very hot path on some loads, so it is
entirely possible that the inlining here is noticeable. I$ patterns in
particular can be a real thing.
(There was an additional issue of just making those user accesses -
get, put and cmpxchg - look a bit more similar)
Linus
On 2025-10-28 11:56, Thomas Gleixner wrote:
> On Tue, Oct 28 2025 at 10:24, Mathieu Desnoyers wrote:
>> On 2025-10-27 04:44, Thomas Gleixner wrote:
>>> From: Thomas Gleixner <tglx@linutronix.de>
>>>
>>> Replace the open coded implementation with the new get/put_user_inline()
>>> helpers. This might be replaced by a regular get/put_user(), but that needs
>>> a proper performance evaluation.
>>
>> I understand that this is aiming to keep the same underlying code,
>> but I find it surprising that the first user of the "inline" get/put
>> user puts the burden of the proof on moving this to regular
>> get/put_user() rather than on using the inlined version.
>>
>> The comment above the inline API clearly states that performance
>> numbers are needed to justify the use of inline, not the opposite.
>>
>> I am concerned that this creates a precedent that may be used by future
>> users of the inline API to use it without performance numbers
>> justification.
>
> There was not justification for the open coded inline either and
> converting it to get/put must be a completely seperate change.
AFAIU there was justification from Linus in
commit 43a43faf5376 ("futex: improve user space accesses").
Perhaps it's good enough to refer to that prior justification
in the commit message.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
The following commit has been merged into the core/rseq branch of tip:
Commit-ID: e4e28fd6986e8cf963ec4137e6c0b95403f636ab
Gitweb: https://git.kernel.org/tip/e4e28fd6986e8cf963ec4137e6c0b95403f636ab
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 27 Oct 2025 09:44:00 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 04 Nov 2025 08:28:23 +01:00
futex: Convert to get/put_user_inline()
Replace the open coded implementation with the new get/put_user_inline()
helpers. This might be replaced by a regular get/put_user(), but that needs
a proper performance evaluation.
No functional change intended.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://patch.msgid.link/20251027083745.736737934@linutronix.de
---
kernel/futex/core.c | 4 +--
kernel/futex/futex.h | 58 ++-----------------------------------------
2 files changed, 5 insertions(+), 57 deletions(-)
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 125804f..ebcccb1 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -581,7 +581,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
if (flags & FLAGS_NUMA) {
u32 __user *naddr = (void *)uaddr + size / 2;
- if (futex_get_value(&node, naddr))
+ if (get_user_inline(node, naddr))
return -EFAULT;
if ((node != FUTEX_NO_NODE) &&
@@ -601,7 +601,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
node = numa_node_id();
node_updated = true;
}
- if (node_updated && futex_put_value(node, naddr))
+ if (node_updated && put_user_inline(node, naddr))
return -EFAULT;
}
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 2cd5709..30c2afa 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -281,63 +281,11 @@ static inline int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32
return ret;
}
-/*
- * This does a plain atomic user space read, and the user pointer has
- * already been verified earlier by get_futex_key() to be both aligned
- * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
- *
- * We still want to avoid any speculation, and while __get_user() is
- * the traditional model for this, it's actually slower than doing
- * this manually these days.
- *
- * We could just have a per-architecture special function for it,
- * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
- * than force everybody to do that, write it out long-hand using
- * the low-level user-access infrastructure.
- *
- * This looks a bit overkill, but generally just results in a couple
- * of instructions.
- */
-static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
-{
- u32 val;
-
- if (can_do_masked_user_access())
- from = masked_user_access_begin(from);
- else if (!user_read_access_begin(from, sizeof(*from)))
- return -EFAULT;
- unsafe_get_user(val, from, Efault);
- user_read_access_end();
- *dest = val;
- return 0;
-Efault:
- user_read_access_end();
- return -EFAULT;
-}
-
-static __always_inline int futex_put_value(u32 val, u32 __user *to)
-{
- if (can_do_masked_user_access())
- to = masked_user_access_begin(to);
- else if (!user_write_access_begin(to, sizeof(*to)))
- return -EFAULT;
- unsafe_put_user(val, to, Efault);
- user_write_access_end();
- return 0;
-Efault:
- user_write_access_end();
- return -EFAULT;
-}
-
+/* Read from user memory with pagefaults disabled */
static inline int futex_get_value_locked(u32 *dest, u32 __user *from)
{
- int ret;
-
- pagefault_disable();
- ret = futex_get_value(dest, from);
- pagefault_enable();
-
- return ret;
+ guard(pagefault)();
+ return get_user_inline(*dest, from);
}
extern void __futex_unqueue(struct futex_q *q);
The following commit has been merged into the core/rseq branch of tip:
Commit-ID: 103ee619d2955cbddab3d13c87113aa9a42601c5
Gitweb: https://git.kernel.org/tip/103ee619d2955cbddab3d13c87113aa9a42601c5
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 27 Oct 2025 09:44:00 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Mon, 03 Nov 2025 15:26:11 +01:00
futex: Convert to get/put_user_inline()
Replace the open coded implementation with the new get/put_user_inline()
helpers. This might be replaced by a regular get/put_user(), but that needs
a proper performance evaluation.
No functional change intended
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20251027083745.736737934@linutronix.de
---
kernel/futex/core.c | 4 +--
kernel/futex/futex.h | 58 ++-----------------------------------------
2 files changed, 5 insertions(+), 57 deletions(-)
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 125804f..ebcccb1 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -581,7 +581,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
if (flags & FLAGS_NUMA) {
u32 __user *naddr = (void *)uaddr + size / 2;
- if (futex_get_value(&node, naddr))
+ if (get_user_inline(node, naddr))
return -EFAULT;
if ((node != FUTEX_NO_NODE) &&
@@ -601,7 +601,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
node = numa_node_id();
node_updated = true;
}
- if (node_updated && futex_put_value(node, naddr))
+ if (node_updated && put_user_inline(node, naddr))
return -EFAULT;
}
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 2cd5709..30c2afa 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -281,63 +281,11 @@ static inline int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32
return ret;
}
-/*
- * This does a plain atomic user space read, and the user pointer has
- * already been verified earlier by get_futex_key() to be both aligned
- * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
- *
- * We still want to avoid any speculation, and while __get_user() is
- * the traditional model for this, it's actually slower than doing
- * this manually these days.
- *
- * We could just have a per-architecture special function for it,
- * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
- * than force everybody to do that, write it out long-hand using
- * the low-level user-access infrastructure.
- *
- * This looks a bit overkill, but generally just results in a couple
- * of instructions.
- */
-static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
-{
- u32 val;
-
- if (can_do_masked_user_access())
- from = masked_user_access_begin(from);
- else if (!user_read_access_begin(from, sizeof(*from)))
- return -EFAULT;
- unsafe_get_user(val, from, Efault);
- user_read_access_end();
- *dest = val;
- return 0;
-Efault:
- user_read_access_end();
- return -EFAULT;
-}
-
-static __always_inline int futex_put_value(u32 val, u32 __user *to)
-{
- if (can_do_masked_user_access())
- to = masked_user_access_begin(to);
- else if (!user_write_access_begin(to, sizeof(*to)))
- return -EFAULT;
- unsafe_put_user(val, to, Efault);
- user_write_access_end();
- return 0;
-Efault:
- user_write_access_end();
- return -EFAULT;
-}
-
+/* Read from user memory with pagefaults disabled */
static inline int futex_get_value_locked(u32 *dest, u32 __user *from)
{
- int ret;
-
- pagefault_disable();
- ret = futex_get_value(dest, from);
- pagefault_enable();
-
- return ret;
+ guard(pagefault)();
+ return get_user_inline(*dest, from);
}
extern void __futex_unqueue(struct futex_q *q);
The following commit has been merged into the core/rseq branch of tip:
Commit-ID: 536e6a70108b609e36866efe8c401d1ed32759ea
Gitweb: https://git.kernel.org/tip/536e6a70108b609e36866efe8c401d1ed32759ea
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 27 Oct 2025 09:44:00 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 29 Oct 2025 11:07:10 +01:00
futex: Convert to get/put_user_inline()
Replace the open coded implementation with the new get/put_user_inline()
helpers. This might be replaced by a regular get/put_user(), but that needs
a proper performance evaluation.
No functional change intended
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20251027083745.736737934@linutronix.de
---
kernel/futex/core.c | 4 +--
kernel/futex/futex.h | 58 ++-----------------------------------------
2 files changed, 5 insertions(+), 57 deletions(-)
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 125804f..ebcccb1 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -581,7 +581,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
if (flags & FLAGS_NUMA) {
u32 __user *naddr = (void *)uaddr + size / 2;
- if (futex_get_value(&node, naddr))
+ if (get_user_inline(node, naddr))
return -EFAULT;
if ((node != FUTEX_NO_NODE) &&
@@ -601,7 +601,7 @@ int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
node = numa_node_id();
node_updated = true;
}
- if (node_updated && futex_put_value(node, naddr))
+ if (node_updated && put_user_inline(node, naddr))
return -EFAULT;
}
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 2cd5709..30c2afa 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -281,63 +281,11 @@ static inline int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32
return ret;
}
-/*
- * This does a plain atomic user space read, and the user pointer has
- * already been verified earlier by get_futex_key() to be both aligned
- * and actually in user space, just like futex_atomic_cmpxchg_inatomic().
- *
- * We still want to avoid any speculation, and while __get_user() is
- * the traditional model for this, it's actually slower than doing
- * this manually these days.
- *
- * We could just have a per-architecture special function for it,
- * the same way we do futex_atomic_cmpxchg_inatomic(), but rather
- * than force everybody to do that, write it out long-hand using
- * the low-level user-access infrastructure.
- *
- * This looks a bit overkill, but generally just results in a couple
- * of instructions.
- */
-static __always_inline int futex_get_value(u32 *dest, u32 __user *from)
-{
- u32 val;
-
- if (can_do_masked_user_access())
- from = masked_user_access_begin(from);
- else if (!user_read_access_begin(from, sizeof(*from)))
- return -EFAULT;
- unsafe_get_user(val, from, Efault);
- user_read_access_end();
- *dest = val;
- return 0;
-Efault:
- user_read_access_end();
- return -EFAULT;
-}
-
-static __always_inline int futex_put_value(u32 val, u32 __user *to)
-{
- if (can_do_masked_user_access())
- to = masked_user_access_begin(to);
- else if (!user_write_access_begin(to, sizeof(*to)))
- return -EFAULT;
- unsafe_put_user(val, to, Efault);
- user_write_access_end();
- return 0;
-Efault:
- user_write_access_end();
- return -EFAULT;
-}
-
+/* Read from user memory with pagefaults disabled */
static inline int futex_get_value_locked(u32 *dest, u32 __user *from)
{
- int ret;
-
- pagefault_disable();
- ret = futex_get_value(dest, from);
- pagefault_enable();
-
- return ret;
+ guard(pagefault)();
+ return get_user_inline(*dest, from);
}
extern void __futex_unqueue(struct futex_q *q);
© 2016 - 2026 Red Hat, Inc.