[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260120081136-f413a6d5-05a5-4adc-a687-474ae349e771@linutronix.de>
Date: Tue, 20 Jan 2026 08:23:39 +0100
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: David Laight <david.laight.linux@...il.com>
Cc: Arnd Bergmann <arnd@...db.de>,
"David S . Miller" <davem@...emloft.net>, Andreas Larsson <andreas@...sler.com>,
Andy Lutomirski <luto@...nel.org>, Thomas Gleixner <tglx@...nel.org>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>, sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org,
Linux-Arch <linux-arch@...r.kernel.org>, linux-s390@...r.kernel.org
Subject: Re: [PATCH 4/4] asm-generic/bitsperlong.h: Add sanity checks for
__BITS_PER_LONG
On Mon, Jan 19, 2026 at 05:47:30PM +0000, David Laight wrote:
> On Mon, 19 Jan 2026 15:57:49 +0100
> "Arnd Bergmann" <arnd@...db.de> wrote:
>
> > On Mon, Jan 19, 2026, at 14:41, Thomas Weißschuh wrote:
> > > On Mon, Jan 19, 2026 at 01:45:04PM +0100, Arnd Bergmann wrote:
> > >> On Mon, Jan 19, 2026, at 11:56, Thomas Weißschuh wrote:
> > >> > On Mon, Jan 19, 2026 at 10:37:58AM +0000, David Laight wrote:
> > >> >>
> > >> >> Don't you need a check that it isn't wrong on a user system?
> > >> >> Which is what I thought it was doing.
> > >> >
> > >> > Not really. The overrides defined by arch/*/include/uapi/asm/bitsperlong.h are
> > >> > being tested here. If they work in the kernel build I assume they also work
> > >> > in userspace.
> > >>
> > >> I think You could just move check into include/asm-generic/bitsperlong.h
> > >> to make this more obvious with the #ifdef __KERNEL__, and remove the
> > >> disabled check from my original version there.
> > >
> > > Ok. I'd like to keep your existing test though, as it tests something different
> > > and it would be nice to have that too at some point.
> >
> > Sure, that works too. I wonder if one of the recent vdso cleanups
> > also happened to address the problem with the incorrect BITS_PER_LONG
> > being visible in the vdso code. Maybe we can already turn that on again.
>
> There is vdso/bits.h, but everything actually includes linux/bits.h first.
These cleanups do not help unfortunately. We can skip the check for BUILD_VDSO,
but there are still plenty other places where it will break.
> I was wondering what happens if you are actually using the 'uapi' headers
> to build programs (may nolibc ones).
> On x86-64, 'gcc foo.c' might work, but 'gcc -m32 foo.c' will find exactly
> the same headers and go badly wrong unless everything is based on
> compiler defines.
I can't follow. __BITS_PER_LONG automatically adapts.
>From arch/x86/include/uapi/asm/bitsperlong.h:
#if defined(__x86_64__) && !defined(__ILP32__)
# define __BITS_PER_LONG 64
#else
# define __BITS_PER_LONG 32
#endif
BITS_PER_LONG on the other hand is never exposed in the uapi headers.
> An assert (of some kind) that checks the pre-processor BITS_PER_LONG
> constant actually matches sizof (long) seems reasonable for all build.
> The alternative is to (somehow) manage to avoid needing a pre-processor
> constant at all, moving everything to 'integer constant expressions'
> instead (good luck with that...).
We do have exactly that assertion in include/asm-generic/bitsperlong.h.
(As mentioned in the patch description we are discussing)
The assertion is disabled because it fails. There are multiple places where
we build 32-bit code with a 64-bit kernel configuration.
The compat vDSO, early boot code and probably more I don't know about.
Is it ugly? Yes. Should we '#define BITS_PER_LONG __BITS_PER_LONG' at some
point? Yes. But we first need to audit all the users to check if it is safe
to do so. At some point in the future in another series.
> I'm most of the way through a 'de-bloat' patchset for bits.h.
> I'm sure there is a good reason why GENMASK(hi, lo) isn't defined
> as '((type)2 << hi) - ((type)1 << lo)'.
> Since that definition doesn't need the bit-width in any form.
> (Just beat up any static checker that objects to '2 << hi' being zero.)
> I've only made that change for ASM files - IIRC the assembler only
> supports one size of signed integer.
No idea, and I don't see the relation to the patches under discussion.
Thomas
Powered by blists - more mailing lists