linux-kernel - Re: [PATCH next 12/14] bits: move the defitions of BIT() and BIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYgP00w2xzWfbx8l@yury>
Date: Sat, 7 Feb 2026 23:23:47 -0500
From: Yury Norov <ynorov@...dia.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: david.laight.linux@...il.com, Nathan Chancellor <nathan@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Arnd Bergmann <arnd@...db.de>, linux-arch@...r.kernel.org,
	linux-kernel@...r.kernel.org, Yury Norov <yury.norov@...il.com>,
	Lucas De Marchi <lucas.demarchi@...el.com>,
	Jani Nikula <jani.nikula@...el.com>,
	Vincent Mailhol <mailhol.vincent@...adoo.fr>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Kees Cook <keescook@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH next 12/14] bits: move the defitions of BIT() and
 BIT_ULL() back to linux/bits.h

On Sat, Feb 07, 2026 at 11:40:41PM +0100, Thomas Gleixner wrote:
> On Wed, Jan 21 2026 at 14:57, david laight linux wrote:
> 
> TLDR: Not going to happen. Period.
> 
> > Move BIT_ULL() and make code that include both headers use the definition
> > of BIT() from linux/bits.h
> > Add BIT_U128() for completness.
> 
> 1) How is that related to $Subject?
> 
> 2) It's clearly documented that patches should not do different things
>    at once.
> 
> 3) It's also documented that stuff is only added when there is a use
>    case. I can't find one.
> 
> > Note that nothing the the x86-64 build relies on the definition in
> > vdso/bits.h, linux/bits.h is always included.
> 
> Wrong. The x86-64 build includes vdso/bits.h for the VDSO build.
> 
> > @@ -2,7 +2,6 @@
> >  #ifndef __LINUX_BITS_H
> >  #define __LINUX_BITS_H
> >  
> > -#include <vdso/bits.h>
> >  #include <uapi/linux/bits.h>
> >  
> >  #define BIT_MASK(nr)		(UL(1) << ((nr) % BITS_PER_LONG))
> > @@ -89,10 +88,16 @@ int BIT_INPUT_CHECK_FAIL(void) __compiletime_error("Bit number out of range");
> >  	((unsigned int)BIT_INPUT_CHECK(+(nr), BITS_PER_TYPE(type)) + ((type)1 << (nr)))
> >  #endif /* defined(__ASSEMBLY__) */
> >  
> > +/* Prefer this definition of BIT() to the one in vdso/bits.h */
> > +#undef BIT
> 
> That's a horrible sloppy hack.
> 
> > +#define __VDSO_BITS_H
> 
> And this even more so.
> 
> > +#define BIT(nr)		BIT_TYPE(unsigned long, nr)
> > +#define BIT_ULL(nr)	BIT_TYPE(unsigned long long, nr)
> 
> Aside of that you sloppily kept the comment above all of this intact,
> which does not make any sense at all after this change. It says:
> 
> /*
>  * Fixed-type variants of BIT(), with additional checks like GENMASK_TYPE(). The
>  * following examples generate compiler warnings due to -Wshift-count-overflow:
>  *
>  * - BIT_U8(8)
>  * - BIT_U32(-1)
>  * - BIT_U32(40)
>  */
> 
> I have to admit that you are at least consistently sloppy.
> 
> >  #define BIT_U8(nr)	BIT_TYPE(u8, nr)
> >  #define BIT_U16(nr)	BIT_TYPE(u16, nr)
> >  #define BIT_U32(nr)	BIT_TYPE(u32, nr)
> >  #define BIT_U64(nr)	BIT_TYPE(u64, nr)
> > +#define BIT_U128(nr)	BIT_TYPE(u128, nr)
> 
> What's wrong with the obvious solution of moving all this BIT_XX() muck
> into vdso/bit.h?
> 
> Especially as you say in your changelog word salad:

I'm next to that. I'm having hard times struggling through this
wording style. As a non-native English speaker, I used to ground
myself when I feel like I can't understand things. But here it's
clearly not only me.

David, for the next iteration, please reword your commit messages and
comments with a more standard version of English.

Regarding this patch, I agree with everything Thomas Gleixner and
Thomas Weißschuh said. This is an NAK.

> > This lets BIT() pick up the extra compile time checks for W=[1c] builds
> > that detect errors like:
> > 	long foo(void) { int x = 64; return BIT(x); }
> > For which clang (silently) just generates a 'return' instruction.
> 
> Letting the VDSO build have the same checks with a W=1 build would be
> too sensible, right?
> 
> It's not rocket science to achieve that. See below.
> 
> That's admittedly a hack too, but a more palatable hack and I'm just
> including it for illustration.
> 
> Just for the record: I definitely spent less time hacking that up than I
> wasted reviewing and replying to your slop.
> 
> The proper thing to do is to move all the stuff which is neither vdso
> nor kernel specific into a separate include/$BIKESHEDTHENAME/ directory
> and sort out that ever recurring problem of VDSO vs. kernel builds once
> and forever. That's not rocket science either.

I would say, "both vdso and kernel", and I would probably add uapi in
the bucket.

Thanks,
Yury

> That'd be too reasonable and tasteful, but not convoluted and sloppy
> enough, right?
> 
> Thanks,
> 
>         tglx