[PATCH next 00/14] bits: De-bloat expansion of GENMASK()

[PATCH next 00/14] bits: De-bloat expansion of GENMASK()

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

The expansion of GENMASK() is a few hundred bytes, this is often multiplied
when the value is passed to other #defines (eg FIELD_PREP).
Part of the size is due to the compile-type check (for reversed arguments),
the rest from the way the value is defined.

Nothing in these patches changes the code the compiler sees - just the
way the constants get defined.
Changing GENMASK(hi, lo) to (2 << hi) - (1 << lo) is left for further study.

I looked at getting the compiler to check for reversed arguments using
(0 >> (hi - lo)) instead of const_true(lo > hi). While checking that it
was always optimised away I discovered that you don't get an error message
if the values are only 'compile time constants', worse clang starts
throwing code away, generation an empty function for:
	int f(u32 x) {int n = 32; return x >> n; }
(Shifts by more than width are 'undefined behaviour', so what clang
does is technically valid - but not friendly or expected.)

So I added extra checks to both GENMASK() and BITxxx() to detect this
at compile time. But this bloats the output - the opposite of what I
was trying to achieve.
However these are all compile-time checks that are actually unlikely
to detect anything, they don't need to be done on every build.
I've mitigated this by adding W=c (cf W=[123e]) to the main Makefile
(adding -DKBUILD_EXTRA_WARNc) and defaulting to W=c for W=1 builds.

Adding checks to BIT() makes it no longer a pre-processor constant
so can no longer be used in #if statements (when W=c) is set.
This required minor changes to 3 files.

At some point the definition of BIT() was moved to vdso/bits.h
(followed by that for BIT_ULL()), but then the fixed size BIT_Unn()
were added to bits.h.
I've moved BIT_ULL() back to linux/bits.h and made the version of
BIT() in linux/bits.h be preferred if both files get included.
Note that the x86-64 allmodconfig build suceeds if vdso/bits.h
is empty - everything includes linux/bits.h first.

I found two non-vdso files that included vdso/bits.h and changed
them to use linux/bits.h.

GENMASK_U8() and BIT_U8() cast their result to (u8), this isn't
a good idea. While the 'type of the expression' is 'u8', integer
promotion makes the 'type of the value' 'signed int'.
This means that in code like:
	u64 v = BIT_U8(7) << 24;
the value is sign extended and all the high bits are set.
Instead change the type of the xxx_U8/U16 macros to 'unsigned int'
so that the sign extension cannot happen.
The compile-time check on the bit number is still present.

For assembler files where GENMASK() can be used for constants
the expansions from uapi/linux/bits.h were used.
However these contain BITS_PER_LONG and BITS_PER_LONG_LONG which
make no sense since the assembler doesn't have sized arithmetic.
Replace with GENMASK(hi, lo) (2 << (hi)) - (1 << (lo)) which has
the correct value without knowing the size of the integers.

The kunit tests all check compile-time values.
I've changed them to use BUILD_BUG_ON().

David Laight (14):
  overflow: Reduce expansion of __type_max()
  kbuild: Add W=c for additional compile time checks
  media: videobuf2-core: Use static_assert() for sanity check
  media: atomisp: Use static_assert() for sanity check
  ixgbevf: Use C test for PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD
  asm-generic: include linux/bits.h not vdso/bits.h
  x86/tlb: include linux/bits.h not vdso/bits.h
  bits: simplify GENMASK_TYPE()
  bits: Change BIT_U8/16() and GENMASK_U8/16() to have unsigned values
  bits: Fix assmebler expansions of GENMASK_Uxx() and BIT_Uxx()
  bit: Strengthen compile-time tests in GENMASK() and BIT()
  bits: move the defitions of BIT() and BIT_ULL() back to linux/bits.h
  test_bits: Change all the tests to be compile-time tests
  test_bits: include some invalid input tests for GENMASK_INPUT_CHECK()

 arch/x86/include/asm/tlb.h                    |   2 +-
 .../media/common/videobuf2/videobuf2-core.c   |   6 +-
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |  17 +--
 .../fixedbds_1.0/ia_css_fixedbds_param.h      |   5 +-
 include/asm-generic/thread_info_tif.h         |   2 +-
 include/linux/bits.h                          |  88 ++++++++----
 include/linux/overflow.h                      |   2 +-
 include/vdso/bits.h                           |   2 +-
 lib/tests/test_bits.c                         | 130 +++++++++++-------
 scripts/Makefile.warn                         |  12 +-
 10 files changed, 162 insertions(+), 104 deletions(-)

-- 
2.39.5

[PATCH next 05/14] ixgbevf: Use C test for PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

Compile-time tests being added to BIT() make it an 'integer constant
expression' rather than a pre-processor expression for W=1 builds.

This means BIT() can't be used in pre-processor conditional that
checks 'PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD'.
Change to use a normal 'if' statement, the compiler will optimise
away the unwanted code.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c   | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index d5ce20f47def..65dd5834d0cf 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4166,9 +4166,6 @@ static int ixgbevf_xmit_frame_ring(struct sk_buff *skb,
 	u32 tx_flags = 0;
 	u16 count = TXD_USE_COUNT(skb_headlen(skb));
 	struct ixgbevf_ipsec_tx_data ipsec_tx = { 0 };
-#if PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD
-	unsigned short f;
-#endif
 	u8 hdr_len = 0;
 	u8 *dst_mac = skb_header_pointer(skb, 0, 0, NULL);
 
@@ -4183,15 +4180,15 @@ static int ixgbevf_xmit_frame_ring(struct sk_buff *skb,
 	 *       + 1 desc for context descriptor,
 	 * otherwise try next time
 	 */
-#if PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD
-	for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
-		skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
+	if (PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD) {
+		for (unsigned int f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
+			skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
 
-		count += TXD_USE_COUNT(skb_frag_size(frag));
+			count += TXD_USE_COUNT(skb_frag_size(frag));
+		}
+	} else {
+		count += skb_shinfo(skb)->nr_frags;
 	}
-#else
-	count += skb_shinfo(skb)->nr_frags;
-#endif
 	if (ixgbevf_maybe_stop_tx(tx_ring, count + 3)) {
 		tx_ring->tx_stats.tx_busy++;
 		return NETDEV_TX_BUSY;
-- 
2.39.5

Re: [PATCH next 05/14] ixgbevf: Use C test for PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD

[Simon Horman]

On Wed, Jan 21, 2026 at 02:57:22PM +0000, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
> 
> Compile-time tests being added to BIT() make it an 'integer constant
> expression' rather than a pre-processor expression for W=1 builds.
> 
> This means BIT() can't be used in pre-processor conditional that
> checks 'PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD'.
> Change to use a normal 'if' statement, the compiler will optimise
> away the unwanted code.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>

Thanks David,

Other than the motivation above, I appreciate that this removes two
#ifdefs, improving readability (subjective) and compile coverage
(objective) of this code.

As an aside: I'm Not sure what your merge plan is for this patchset, and
only it and the cover letter hit my inbox, so I'm missing context.  But
from a Networking PoV it seems that it could be sent as a stand-alone patch
to the iwl tree.

Regardless, feel free to add:

Reviewed-by: Simon Horman <horms@kernel.org>

...