tools/testing/selftests/futex/include/futex2test.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Linus,
please pull the latest locking/urgent branch from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-urgent-2025-07-20
up to: d0a48dc4df5c: selftests/futex: Convert 32-bit timespec to 64-bit version for 32-bit compatibility mode
A single fix for the futex selftest code to make 32-bit user space work
correctly on 64-bit kernels. sys_futex_wait() expects a struct
__kernel_timespec for the timeout, but the selftest uses struct timespec,
which is the original 32-bit non 2038 compliant variant. Fix it up by
converting the callsite supplied timespec to a __kernel_timespec and hand
that into the syscall.
Thanks,
tglx
------------------>
Terry Tritton (1):
selftests/futex: Convert 32-bit timespec to 64-bit version for 32-bit compatibility mode
tools/testing/selftests/futex/include/futex2test.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/futex/include/futex2test.h b/tools/testing/selftests/futex/include/futex2test.h
index ea79662405bc..1f625b39948a 100644
--- a/tools/testing/selftests/futex/include/futex2test.h
+++ b/tools/testing/selftests/futex/include/futex2test.h
@@ -4,6 +4,7 @@
*
* Copyright 2021 Collabora Ltd.
*/
+#include <linux/time_types.h>
#include <stdint.h>
#define u64_to_ptr(x) ((void *)(uintptr_t)(x))
@@ -65,7 +66,12 @@ struct futex32_numa {
static inline int futex_waitv(volatile struct futex_waitv *waiters, unsigned long nr_waiters,
unsigned long flags, struct timespec *timo, clockid_t clockid)
{
- return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, timo, clockid);
+ struct __kernel_timespec ts = {
+ .tv_sec = timo->tv_sec,
+ .tv_nsec = timo->tv_nsec,
+ };
+
+ return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, &ts, clockid);
}
/*The pull request you sent on Sun, 20 Jul 2025 14:04:57 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-urgent-2025-07-20 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/92329d578d60fe944e30da287ee28f5c154a5802 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Linus,
please pull the latest sched/urgent branch from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched-urgent-2025-07-20
up to: 36569780b0d6: sched: Change nr_uninterruptible type to unsigned long
A single fix for the scheduler. A recent commit changed the runqueue
counter nr_uninterruptible to an unsigned int. Due to the fact that the
counters are not updated on migration of a uninterruptble task to a
different CPU, these counters can exceed INT_MAX. The counter is cast to
long in the load average calculation, which means that the cast expands
into negative space resulting in bogus load average values. Convert it back
to unsigned long to fix this.
Thanks,
tglx
------------------>
Aruna Ramakrishna (1):
sched: Change nr_uninterruptible type to unsigned long
kernel/sched/loadavg.c | 2 +-
kernel/sched/sched.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c
index c48900b856a2..52ca8e268cfc 100644
--- a/kernel/sched/loadavg.c
+++ b/kernel/sched/loadavg.c
@@ -80,7 +80,7 @@ long calc_load_fold_active(struct rq *this_rq, long adjust)
long nr_active, delta = 0;
nr_active = this_rq->nr_running - adjust;
- nr_active += (int)this_rq->nr_uninterruptible;
+ nr_active += (long)this_rq->nr_uninterruptible;
if (nr_active != this_rq->calc_load_active) {
delta = nr_active - this_rq->calc_load_active;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 475bb5998295..83e3aa917142 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1149,7 +1149,7 @@ struct rq {
* one CPU and if it got migrated afterwards it may decrease
* it on another CPU. Always updated under the runqueue lock:
*/
- unsigned int nr_uninterruptible;
+ unsigned long nr_uninterruptible;
union {
struct task_struct __rcu *donor; /* Scheduler context */The pull request you sent on Sun, 20 Jul 2025 14:04:59 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched-urgent-2025-07-20 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/62347e279092ae704877467abdc8533e914f945e Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
On Sun, 20 Jul 2025 at 05:05, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> A single fix for the scheduler. A recent commit changed the runqueue
> counter nr_uninterruptible to an unsigned int. Due to the fact that the
> counters are not updated on migration of a uninterruptble task to a
> different CPU, these counters can exceed INT_MAX. The counter is cast to
> long in the load average calculation, which means that the cast expands
> into negative space resulting in bogus load average values. Convert it back
> to unsigned long to fix this.
I've obviously pulled this, but considering that the thing isn't
actually really an unsigned value in the first place, and all users
seem to cast it to 'long' anyway, there's an obvious question... Why
is it unsigned in the first place, rather than just make it be the
more natural signed type?
At that point, the _size_ of the type wouldn't even have mattered
(outside of actual overflows, of course).
Of course, regardless of all this, if it does negative due to
migration, it looks like the calc_load_fold_active() calculations end
up being a bit nonsensical. Not that it looks like that matters
either, but it did make me go "Hmm - people seem to _assume_ it's
always positive, it has a _type_ that is always positive, but then we
have those odd hacks for the fact that it can actually be negative".
So none of this seems to _matter_ (outside of the "unsigned int got
*really* mangled" issue that this pull fixes), but it's all a bit odd.
Linus
On Sun, Jul 20, 2025 at 11:20:31AM -0700, Linus Torvalds wrote: > I've obviously pulled this, but considering that the thing isn't > actually really an unsigned value in the first place, and all users > seem to cast it to 'long' anyway, there's an obvious question... Why > is it unsigned in the first place, rather than just make it be the > more natural signed type? Histerical raisins AFAIU. I think the original reason to make the thing unsigned was to get the 2s complement wrapping behaviour. Yes, these days we have -fwrapv and everybody knows signed types wrap properly too (although Kees will argue signed should not wrap etc..). But back then who knows.
Linus,
please pull the latest x86/urgent branch from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-2025-07-20
up to: 6b995d01683f: x86/sev: Work around broken noinstr on GCC
A single fix for a GCC wreckage, which emits a KCSAN instrumentation call
in __sev_es_nmi_complete() despite the function being annotated with
'noinstr'. As all functions in that source file are noinstr, exclude the
whole file from KCSAN in the Makefile to cure it.
Thanks,
tglx
------------------>
Ard Biesheuvel (1):
x86/sev: Work around broken noinstr on GCC
arch/x86/coco/sev/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/coco/sev/Makefile b/arch/x86/coco/sev/Makefile
index db3255b979bd..342d79f0ab6a 100644
--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -5,5 +5,6 @@ obj-y += core.o sev-nmi.o vc-handle.o
# Clang 14 and older may fail to respect __no_sanitize_undefined when inlining
UBSAN_SANITIZE_sev-nmi.o := n
-# GCC may fail to respect __no_sanitize_address when inlining
+# GCC may fail to respect __no_sanitize_address or __no_kcsan when inlining
KASAN_SANITIZE_sev-nmi.o := n
+KCSAN_SANITIZE_sev-nmi.o := nThe pull request you sent on Sun, 20 Jul 2025 14:05:01 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-2025-07-20 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/07fa9cad54609df3eea00cd5b167df6088ce01a6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
On Sun, 20 Jul 2025 at 05:05, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> A single fix for a GCC wreckage, which emits a KCSAN instrumentation call
> in __sev_es_nmi_complete() despite the function being annotated with
> 'noinstr'. As all functions in that source file are noinstr, exclude the
> whole file from KCSAN in the Makefile to cure it.
Hmm. I'm not entirely sure if this counts as a gcc bug.
In particular, look at the *declaration* of __sev_es_nmi_complete() in
<asm/sev.h>, and note the complete lack of 'noinstr':
extern void __sev_es_nmi_complete(void);
and I wonder if it might be the case that gcc might pick up the lack
of 'noinstr' from the declaration, even if it's there in the
definition..
The fix for this obviously ends up working and is fine regardless, but
it's _possible_ that this is self-inflicted pain rather than an
outright compiler bug. Because having a declaration and a definition
that doesn't match sounds like a bad idea in the first place.
Linus
On Sun, Jul 20 2025 at 11:35, Linus Torvalds wrote:
> On Sun, 20 Jul 2025 at 05:05, Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> A single fix for a GCC wreckage, which emits a KCSAN instrumentation call
>> in __sev_es_nmi_complete() despite the function being annotated with
>> 'noinstr'. As all functions in that source file are noinstr, exclude the
>> whole file from KCSAN in the Makefile to cure it.
>
> Hmm. I'm not entirely sure if this counts as a gcc bug.
>
> In particular, look at the *declaration* of __sev_es_nmi_complete() in
> <asm/sev.h>, and note the complete lack of 'noinstr':
>
> extern void __sev_es_nmi_complete(void);
>
> and I wonder if it might be the case that gcc might pick up the lack
> of 'noinstr' from the declaration, even if it's there in the
> definition..
>
> The fix for this obviously ends up working and is fine regardless, but
> it's _possible_ that this is self-inflicted pain rather than an
> outright compiler bug.
I agree. See below.
> Because having a declaration and a definition that doesn't match
> sounds like a bad idea in the first place.
I disagree here. When the compiler evaluates the function body it must
have evaluated noinstr on the definition, no?
Unfortunately the data provided in the change log does not tell what was
actually instrumented inside of that function. I just reproduced it
locally.
The problem are the invocations of ghcb_set_sw_*(), which are
implemented through a macro maze, but end up being marked
__always_inline all the way down. __always_inline is supposed to
guarantee that the noinstr (or other) constraints of the calling
function are preserved.
What makes these ghcb_set_sw_*() inlines so special?
They are defined by DEFINE_GHCB_ACCESSORS(field) and end up with this:
static __always_inline void ghcb_set_##field(struct ghcb *ghcb, u64 value) \
{ \
__set_bit(GHCB_BITMAP_IDX(field), \
(unsigned long *)&ghcb->save.valid_bitmap); \
ghcb->save.field = value; \
}
__set_bit() resolves to:
static __always_inline void ___set_bit(unsigned long nr, volatile unsigned long *addr)
{
instrument_write(addr + BIT_WORD(nr), sizeof(long));
arch___set_bit(nr, addr);
}
instrument_write() resolves to
static __always_inline void instrument_write(const volatile void *v, size_t size)
{
kasan_check_write(v, size);
kcsan_check_write(v, size);
}
and kcsan_check_write() is:
#define __kcsan_check_write(ptr, size) \
__kcsan_check_access(ptr, size, KCSAN_ACCESS_WRITE)
__kcsan_check_access() is a function provided by the kernel. So GCC just
does as told and emits a function call.
If I replace the __set_bit() in ghcb_set_##field() with arch___set_bit()
the problem goes away.
Excluding the whole file from KCSAN switches to the non-instrumented
version of __set_bit() which directly resolves to arch___set_bit().
Thanks,
tglx
(cc Marco, Daniel)
Hi Thomas,
Thanks for getting to the bottom of this.
On Mon, 21 Jul 2025 at 16:08, tglx <tglx@linutronix.de> wrote:
>
> On Sun, Jul 20 2025 at 11:35, Linus Torvalds wrote:
> > On Sun, 20 Jul 2025 at 05:05, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> A single fix for a GCC wreckage, which emits a KCSAN instrumentation call
> >> in __sev_es_nmi_complete() despite the function being annotated with
> >> 'noinstr'. As all functions in that source file are noinstr, exclude the
> >> whole file from KCSAN in the Makefile to cure it.
> >
> > Hmm. I'm not entirely sure if this counts as a gcc bug.
> >
> > In particular, look at the *declaration* of __sev_es_nmi_complete() in
> > <asm/sev.h>, and note the complete lack of 'noinstr':
> >
> > extern void __sev_es_nmi_complete(void);
> >
> > and I wonder if it might be the case that gcc might pick up the lack
> > of 'noinstr' from the declaration, even if it's there in the
> > definition..
> >
> > The fix for this obviously ends up working and is fine regardless, but
> > it's _possible_ that this is self-inflicted pain rather than an
> > outright compiler bug.
>
> I agree. See below.
>
> > Because having a declaration and a definition that doesn't match
> > sounds like a bad idea in the first place.
>
> I disagree here. When the compiler evaluates the function body it must
> have evaluated noinstr on the definition, no?
>
> Unfortunately the data provided in the change log does not tell what was
> actually instrumented inside of that function. I just reproduced it
> locally.
>
...
> __set_bit() resolves to:
>
> static __always_inline void ___set_bit(unsigned long nr, volatile unsigned long *addr)
> {
> instrument_write(addr + BIT_WORD(nr), sizeof(long));
> arch___set_bit(nr, addr);
> }
>
So this is the issue right here: an __always_inline function that
unconditionally calls the KASAN/KCSAN check functions. And indeed, the
compiler is not to blame here, because these instrumentations were
emitted by our code, and in a manner that doesn't care about the
compiler attributes that disable KASAN or KCSAN.
The upshot of this is that all noinstr users of __set_bit() end up
calling the instrumented version if the kconfig happens to have KASAN
or KCSAN enabled, and I seriously doubt that this is what we want.
Including one header or the other obviously doesn't work when
disabling these instrumentations at the function level.
Ideally, we'd have __builtin_kasan_enabled() and
__builtin_kcsan_enabled() compiler builtins that provide the correct
answer for the current function, but that will take a while to land if
we start working on it now (for both Clang and GCC).
On Mon, 21 Jul 2025 at 04:35, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sun, 20 Jul 2025 at 05:05, Thomas Gleixner <tglx@linutronix.de> wrote: > > > > A single fix for a GCC wreckage, which emits a KCSAN instrumentation call > > in __sev_es_nmi_complete() despite the function being annotated with > > 'noinstr'. As all functions in that source file are noinstr, exclude the > > whole file from KCSAN in the Makefile to cure it. > > Hmm. I'm not entirely sure if this counts as a gcc bug. > > In particular, look at the *declaration* of __sev_es_nmi_complete() in > <asm/sev.h>, and note the complete lack of 'noinstr': > > extern void __sev_es_nmi_complete(void); > > and I wonder if it might be the case that gcc might pick up the lack > of 'noinstr' from the declaration, even if it's there in the > definition.. > Just tried this: adding 'noinstr' to the declaration in asm/sev.h makes no difference at all. > The fix for this obviously ends up working and is fine regardless, but > it's _possible_ that this is self-inflicted pain rather than an > outright compiler bug. Because having a declaration and a definition > that doesn't match sounds like a bad idea in the first place. > Agree with this in principle, and for 'noinstr' in particular, this may work fine but note that there are __attribute__(()) annotations that make no sense, or are not permitted on [forward] declarations, and so having the general rule that declarations and definitions must have the same annotations may not be feasible in practice.
On Sun, 20 Jul 2025 at 20:14, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Just tried this: adding 'noinstr' to the declaration in asm/sev.h
> makes no difference at all.
Ok, thanks for checking. It does seem a strange bug.
That said, this area is a mess, and I really do think it's at least
partly *our* mess.
We should mark not only __sev_es_nmi_complete(), but the
sev_es_nmi_complete() inline function wrapper as being 'noinstr'.
But we can't do that, because 'noinstr' explicitly includes
'noinline', so we cannot do something sane like
static __always_inline noinstr void sev_es_nmi_complete(void) ..
for the wrapper, because the compiler will very correctly say "I'm
sorry Dave, I can't do that".
So I still suspect that yes, it may be a gcc bug, but we're really
doing some things that make me go "at some point you really can't
blame the compiler too much for being confused".
I guess it doesn't matter - the bug is fixed, but I'd personally
hesitate to make a gcc bug report simply because if I was a compiler
person, I would take one look at this code and say "you're insane".
Linus
On Mon, 21 Jul 2025 at 13:32, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sun, 20 Jul 2025 at 20:14, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > Just tried this: adding 'noinstr' to the declaration in asm/sev.h > > makes no difference at all. > > Ok, thanks for checking. It does seem a strange bug. > > That said, this area is a mess, and I really do think it's at least > partly *our* mess. > > We should mark not only __sev_es_nmi_complete(), but the > sev_es_nmi_complete() inline function wrapper as being 'noinstr'. > > But we can't do that, because 'noinstr' explicitly includes > 'noinline', so we cannot do something sane like > > static __always_inline noinstr void sev_es_nmi_complete(void) .. > > for the wrapper, because the compiler will very correctly say "I'm > sorry Dave, I can't do that". > I also wonder what the semantics should be in this case, though: attributes are generally tracked at function granularity or coarser, and so inlining a noinstr function should either disable instrumentation for the whole outer function, or disregard the noinstr, as applying it only to the inlined code is not generally possible.
© 2016 - 2025 Red Hat, Inc.