[PATCH 0/6] lib: rework bitreverse

[PATCH 0/6] lib: rework bitreverse

[Yury Norov]

This series is a resend for Jinjie Ruan's "arch/riscv: Add bitrev.h file
to support rev8 and brev8" [1], my follow-up "lib: compile generic
bitrev based on GENERIC_BITREVERSE" [2], and the fix for a build error
reported by Nathan Chancellor [3].

No changes, except for combining pieces together and rebasing on top of
the tree.

[1] https://lore.kernel.org/all/20260421130752.607500-1-ruanjinjie@huawei.com/
[2] https://lore.kernel.org/all/20260427205210.397471-1-ynorov@nvidia.com/
[3] https://lore.kernel.org/all/20260429202922.GA3575295@ax162/

Build-tested against x86 tinyconfig and defconfig, having disabled and
enabiled CRC32 and BITREVERSE, correspondingly.

Jinjie Ruan (3):
  lib/bitrev: Introduce GENERIC_BITREVERSE and cleanup Kconfig
  bitops: Define generic __bitrev8/16/32 for reuse
  arch/riscv: Add bitrev.h file to support rev8 and brev8

Yury Norov (3):
  lib: include crc32.h conditionally on CONFIG_CRC32
  lib: compile generic bitrev.c conditionally on GENERIC_BITREVERSE
  MAINTAINERS: BITOPS: include bitrev.[ch]

 MAINTAINERS                           |  2 ++
 arch/riscv/Kconfig                    |  2 ++
 arch/riscv/include/asm/bitrev.h       | 51 +++++++++++++++++++++++++++
 include/asm-generic/bitops/__bitrev.h | 25 +++++++++++++
 include/linux/bitrev.h                | 20 +++--------
 include/linux/etherdevice.h           |  4 +++
 lib/Kconfig                           | 18 ++++++++++
 lib/Makefile                          |  2 +-
 lib/bitrev.c                          |  3 --
 9 files changed, 107 insertions(+), 20 deletions(-)
 create mode 100644 arch/riscv/include/asm/bitrev.h
 create mode 100644 include/asm-generic/bitops/__bitrev.h

-- 
2.51.0

[PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Yury Norov]

Currently, bitreverse API is either declared based on
CONFIG_HAVE_ARCH_BITREVERSE, wired to arch implementation, or if the
arch has no bitreverse, based on generic implementation.

So, regardless of CONFIG_BITREVERSE=n, the corresponding API is always
declared. If that happens, the functions become declared but not
implemented, which is an error.

The following patches of the series make it possible to have bitreverse
API undeclared if CONFIG_BITREVERSE=n, thus spotting the problem when
building the tinyconfig:

   $ make -skj"$(nproc)" ARCH=s390 CROSS_COMPILE=s390-linux- mrproper tinyconfig fs/select.o
   In file included from include/linux/crc32.h:6,
                    from include/linux/etherdevice.h:23,
                    from include/linux/if_vlan.h:11,
                    from include/linux/filter.h:21,
                    from include/net/xdp.h:10,
                    from include/net/busy_poll.h:19,
                    from fs/select.c:33:
   include/linux/etherdevice.h: In function 'eth_hw_addr_crc':
   include/linux/bitrev.h:16:20: error: implicit declaration of function 'generic___bitrev32' [-Wimplicit-function-declaration]
      16 | #define __bitrev32 generic___bitrev32
         |                    ^~~~~~~~~~~~~~~~~~
   include/linux/bitrev.h:67:9: note: in expansion of macro '__bitrev32'
      67 |         __bitrev32(__x);                                \
         |         ^~~~~~~~~~
   include/linux/crc32.h:107:36: note: in expansion of macro 'bitrev32'
     107 | #define ether_crc(length, data)    bitrev32(crc32_le(~0, data, length))
         |                                    ^~~~~~~~
   include/linux/etherdevice.h:292:16: note: in expansion of macro 'ether_crc'
     292 |         return ether_crc(ETH_ALEN, ha->addr);
         |                ^~~~~~~~~
   make[5]: *** [scripts/Makefile.build:289: fs/select.o] Error 1
   ...

The current unconditionally enabled codebase doesn't use CRC32, neither
bitrev functionality, and if generic___bitrev32 prototype is provided,
the compilation and linkage phases are passed OK.

The only header requiring the crc32 and bitreverse prototypes is
include/linux/etherdevice.h. Thus, protect inclusion of corresponding
headers in the etherdevice with CONFIG_CRC32, together with the only
function depending on it.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 include/linux/etherdevice.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index df8f88f63a70..d35be27a91a5 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -20,7 +20,9 @@
 #include <linux/if_ether.h>
 #include <linux/netdevice.h>
 #include <linux/random.h>
+#ifdef CONFIG_CRC32
 #include <linux/crc32.h>
+#endif
 #include <linux/unaligned.h>
 #include <asm/bitsperlong.h>
 
@@ -281,6 +283,7 @@ static inline void eth_hw_addr_random(struct net_device *dev)
 	dev->addr_assign_type = NET_ADDR_RANDOM;
 }
 
+#ifdef CONFIG_CRC32
 /**
  * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
  * @ha: pointer to hardware address
@@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
 {
 	return ether_crc(ETH_ALEN, ha->addr);
 }
+#endif
 
 /**
  * ether_addr_copy - Copy an Ethernet address
-- 
2.51.0

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Using `#ifdef CONFIG_CRC32` breaks the build for modular drivers that rely on `eth_hw_addr_crc()`.
--

commit 734658b27c8ae06b58089e30d3e39cd61c7d09d6
Author: Yury Norov <ynorov@nvidia.com>

lib: include crc32.h conditionally on CONFIG_CRC32

This commit restricts the inclusion of crc32.h and the definition of
eth_hw_addr_crc() in etherdevice.h to only when CONFIG_CRC32 is enabled.
This fixes implicit declaration errors when building tinyconfig where
CONFIG_BITREVERSE=n.

> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index df8f88f63a706..d35be27a91a55 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -20,7 +20,9 @@
>  #include <linux/if_ether.h>
>  #include <linux/netdevice.h>
>  #include <linux/random.h>
> +#ifdef CONFIG_CRC32
>  #include <linux/crc32.h>
> +#endif

Will this break the build for modular drivers?

CONFIG_CRC32 is a tristate option. When it is configured as a module (=m), the
build system defines CONFIG_CRC32_MODULE but not CONFIG_CRC32. Thus, the
preprocessor check evaluates to false.

Should this use #if IS_ENABLED(CONFIG_CRC32) instead to support both
built-in and modular configurations?

[ ... ]

> @@ -281,6 +283,7 @@ static inline void eth_hw_addr_random(struct net_device *dev)
>  	dev->addr_assign_type = NET_ADDR_RANDOM;
>  }
>  
> +#ifdef CONFIG_CRC32
>  /**
>   * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
>   * @ha: pointer to hardware address
> @@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
>  {
>  	return ether_crc(ETH_ALEN, ha->addr);
>  }
> +#endif

If CONFIG_CRC32 is built as a module, will modular network drivers that call
eth_hw_addr_crc() fail to compile due to an implicit declaration error since
this definition is hidden?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430211351.658193-1-ynorov@nvidia.com?part=1

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Arnd Bergmann]

On Thu, Apr 30, 2026, at 23:13, Yury Norov wrote:
> Currently, bitreverse API is either declared based on
> CONFIG_HAVE_ARCH_BITREVERSE, wired to arch implementation, or if the
> arch has no bitreverse, based on generic implementation.
>
> So, regardless of CONFIG_BITREVERSE=n, the corresponding API is always
> declared. If that happens, the functions become declared but not
> implemented, which is an error.

I'm not following that description. Why is it an error to declare
a funtion that is not implemented? Isn't that how optional interfaces
tend to work in general?

> The only header requiring the crc32 and bitreverse prototypes is
> include/linux/etherdevice.h. Thus, protect inclusion of corresponding
> headers in the etherdevice with CONFIG_CRC32, together with the only
> function depending on it.
...
>  #include <linux/if_ether.h>
>  #include <linux/netdevice.h>
>  #include <linux/random.h>
> +#ifdef CONFIG_CRC32
>  #include <linux/crc32.h>
> +#endif
>  #include <linux/unaligned.h>
>  #include <asm/bitsperlong.h>

Don't add #ifdef blocks around headers. If the header cannot
be included without side-effects, change the linux/crc32.h
file instead of its users.

It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
in include/asm-generic/bitops/__bitrev.h, which ends up
hinding the generic___bitrev32() helper without need.

Simply removing the #ifdef there should avoid the build failure.

> +#ifdef CONFIG_CRC32
>  /**
>   * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
>   * @ha: pointer to hardware address
> @@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
>  {
>  	return ether_crc(ETH_ALEN, ha->addr);
>  }
> +#endif

I see there are only user users of this function, neither of
them are performance critical. So the other options would
be to either open-code this function in the two callers
and remove it entirely, or move it into net/ethernet/eth.c.

      Arnd

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[David Laight]

On Mon, 04 May 2026 10:03:10 +0200
"Arnd Bergmann" <arnd@arndb.de> wrote:

> On Thu, Apr 30, 2026, at 23:13, Yury Norov wrote:
> > Currently, bitreverse API is either declared based on
> > CONFIG_HAVE_ARCH_BITREVERSE, wired to arch implementation, or if the
> > arch has no bitreverse, based on generic implementation.
> >
> > So, regardless of CONFIG_BITREVERSE=n, the corresponding API is always
> > declared. If that happens, the functions become declared but not
> > implemented, which is an error.  
> 
> I'm not following that description. Why is it an error to declare
> a funtion that is not implemented? Isn't that how optional interfaces
> tend to work in general?
> 
> > The only header requiring the crc32 and bitreverse prototypes is
> > include/linux/etherdevice.h. Thus, protect inclusion of corresponding
> > headers in the etherdevice with CONFIG_CRC32, together with the only
> > function depending on it.  
> ...
> >  #include <linux/if_ether.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/random.h>
> > +#ifdef CONFIG_CRC32
> >  #include <linux/crc32.h>
> > +#endif
> >  #include <linux/unaligned.h>
> >  #include <asm/bitsperlong.h>  
> 
> Don't add #ifdef blocks around headers. If the header cannot
> be included without side-effects, change the linux/crc32.h
> file instead of its users.
> 
> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
> in include/asm-generic/bitops/__bitrev.h, which ends up
> hinding the generic___bitrev32() helper without need.
> 
> Simply removing the #ifdef there should avoid the build failure.
> 
> > +#ifdef CONFIG_CRC32
> >  /**
> >   * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
> >   * @ha: pointer to hardware address
> > @@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
> >  {
> >  	return ether_crc(ETH_ALEN, ha->addr);
> >  }
> > +#endif  
> 
> I see there are only user users of this function, neither of
> them are performance critical. So the other options would
> be to either open-code this function in the two callers
> and remove it entirely, or move it into net/ethernet/eth.c.

Or change to a #define so that only the users need to have the
required headers included.

But open-coding in the callers saves anyone trying to read the code
having to look at another file to see what is going on.

-- David

> 
>       Arnd
> 

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Yury Norov]

On Mon, May 04, 2026 at 10:03:10AM +0200, Arnd Bergmann wrote:
> On Thu, Apr 30, 2026, at 23:13, Yury Norov wrote:
> > Currently, bitreverse API is either declared based on
> > CONFIG_HAVE_ARCH_BITREVERSE, wired to arch implementation, or if the
> > arch has no bitreverse, based on generic implementation.
> >
> > So, regardless of CONFIG_BITREVERSE=n, the corresponding API is always
> > declared. If that happens, the functions become declared but not
> > implemented, which is an error.
> 
> I'm not following that description. Why is it an error to declare
> a funtion that is not implemented? Isn't that how optional interfaces
> tend to work in general?

Never heard about such a thing like "optional interface". And git grep
tends to second that...
 
> > The only header requiring the crc32 and bitreverse prototypes is
> > include/linux/etherdevice.h. Thus, protect inclusion of corresponding
> > headers in the etherdevice with CONFIG_CRC32, together with the only
> > function depending on it.
> ...
> >  #include <linux/if_ether.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/random.h>
> > +#ifdef CONFIG_CRC32
> >  #include <linux/crc32.h>
> > +#endif
> >  #include <linux/unaligned.h>
> >  #include <asm/bitsperlong.h>
> 
> Don't add #ifdef blocks around headers. If the header cannot
> be included without side-effects, change the linux/crc32.h
> file instead of its users.

linux/acpi.h does that like many othes. What exactly is wrong with
protecting headers inclusion?
 
> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
> in include/asm-generic/bitops/__bitrev.h, which ends up
> hinding the generic___bitrev32() helper without need.
> 
> Simply removing the #ifdef there should avoid the build failure.

OK, it seems like this is what I don't understand.

We've got an optional feature, like CRC32, which is enabled by
CONFIG_CRC32. The most conservative way is to declare everything
CRC32-related in the corresponding header, and then protect the header
with IS_ENABLED(CONFIG_CRC32).

I understand that from practical perspective, we can declare some simple
macros, like header size, unprotected. But what we've got now is a sort
of mess: all CRC32-related functions are declared unprotected, and
generic headers are good to use them. Compiler is happy while those
functions are actually unused. Next, CRC32 depends on BITREVERSE, which
is again unprotected, and it may optionally have an arch implementation.

So if arch bitrev() is implemented, you can use part of bitreverse and
crc32 APIs despite that they are explicitly disabled - just because they
are implemented as macros in unprotected headers. And you cannot use some
others - because they are implemented differently, as a real functions.

And this is not covered by any written rule - just the implementation
details. And to make it worse, this all is available to drivers, which
may simply fail after the next kernel update.

Can you please elaborate on how is that supposed to work. In my naive
world, if I disable some feature, I'm pretty sure that my kernel
shouldn't build if I try to use it. Can you point to any related
kernel docs?

Thanks,
Yury

> > +#ifdef CONFIG_CRC32
> >  /**
> >   * eth_hw_addr_crc - Calculate CRC from netdev_hw_addr
> >   * @ha: pointer to hardware address
> > @@ -291,6 +294,7 @@ static inline u32 eth_hw_addr_crc(struct netdev_hw_addr *ha)
> >  {
> >  	return ether_crc(ETH_ALEN, ha->addr);
> >  }
> > +#endif
> 
> I see there are only user users of this function, neither of
> them are performance critical. So the other options would
> be to either open-code this function in the two callers
> and remove it entirely, or move it into net/ethernet/eth.c.
> 
>       Arnd

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Arnd Bergmann]

On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
> On Mon, May 04, 2026 at 10:03:10AM +0200, Arnd Bergmann wrote:
>> On Thu, Apr 30, 2026, at 23:13, Yury Norov wrote:
>> I'm not following that description. Why is it an error to declare
>> a funtion that is not implemented? Isn't that how optional interfaces
>> tend to work in general?
>
> Never heard about such a thing like "optional interface". And git grep
> tends to second that...

I meant any library interface that can be turned on or off
 
>> > The only header requiring the crc32 and bitreverse prototypes is
>> > include/linux/etherdevice.h. Thus, protect inclusion of corresponding
>> > headers in the etherdevice with CONFIG_CRC32, together with the only
>> > function depending on it.
>> ...
>> >  #include <linux/if_ether.h>
>> >  #include <linux/netdevice.h>
>> >  #include <linux/random.h>
>> > +#ifdef CONFIG_CRC32
>> >  #include <linux/crc32.h>
>> > +#endif
>> >  #include <linux/unaligned.h>
>> >  #include <asm/bitsperlong.h>
>> 
>> Don't add #ifdef blocks around headers. If the header cannot
>> be included without side-effects, change the linux/crc32.h
>> file instead of its users.
>
> linux/acpi.h does that like many othes. What exactly is wrong with
> protecting headers inclusion?

There is no "protecting" here, you just add complexity to the
build when headers are sometimes included indirectly and but
other times are not, depending on kernel configuration.

It's unlikely to cause problems for the crc32.h header, but
the acpi example definitely risks running into circular
inclusions when you end up with some other header that depending
on configuration ends up including linux/acpi.h while also
bring included indirectly from that one.

>> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
>> in include/asm-generic/bitops/__bitrev.h, which ends up
>> hinding the generic___bitrev32() helper without need.
>> 
>> Simply removing the #ifdef there should avoid the build failure.
>
> OK, it seems like this is what I don't understand.
>
> We've got an optional feature, like CRC32, which is enabled by
> CONFIG_CRC32. The most conservative way is to declare everything
> CRC32-related in the corresponding header, and then protect the header
> with IS_ENABLED(CONFIG_CRC32).
>
> I understand that from practical perspective, we can declare some simple
> macros, like header size, unprotected. But what we've got now is a sort
> of mess: all CRC32-related functions are declared unprotected, and
> generic headers are good to use them. Compiler is happy while those
> functions are actually unused. Next, CRC32 depends on BITREVERSE, which
> is again unprotected, and it may optionally have an arch implementation.
>
> So if arch bitrev() is implemented, you can use part of bitreverse and
> crc32 APIs despite that they are explicitly disabled - just because they
> are implemented as macros in unprotected headers. And you cannot use some
> others - because they are implemented differently, as a real functions.

I think you trying to solve a non-problem here. It is extremely rare
for any kernel to be built without crc32 or bitrev. Even in randconfig
builds, both are usually selected by some driver. The behavior
we've always had here is that in the rare one randconfig in 10000
that hits the one driver missing 'select BITREVERSE', we get a link
failure with an obvious fix. If you add the extra #ifdef, it gets
a little more likely to actually run into the bug, and it shows
up a little earlier as a compile failure instead of a link failure,
but otherwise, nothing changes.

       Arnd

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Yury Norov]

On Mon, May 04, 2026 at 07:18:49PM +0200, Arnd Bergmann wrote:
> On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
> > On Mon, May 04, 2026 at 10:03:10AM +0200, Arnd Bergmann wrote:
> >> On Thu, Apr 30, 2026, at 23:13, Yury Norov wrote:
> >> I'm not following that description. Why is it an error to declare
> >> a funtion that is not implemented? Isn't that how optional interfaces
> >> tend to work in general?
> >
> > Never heard about such a thing like "optional interface". And git grep
> > tends to second that...
> 
> I meant any library interface that can be turned on or off

So? If I disable CRC32, can I use the either_crc()? In case of that
networking header, the answer is yes. In some other piece of code
the answer is no. Is that correct?
  
> >> > The only header requiring the crc32 and bitreverse prototypes is
> >> > include/linux/etherdevice.h. Thus, protect inclusion of corresponding
> >> > headers in the etherdevice with CONFIG_CRC32, together with the only
> >> > function depending on it.
> >> ...
> >> >  #include <linux/if_ether.h>
> >> >  #include <linux/netdevice.h>
> >> >  #include <linux/random.h>
> >> > +#ifdef CONFIG_CRC32
> >> >  #include <linux/crc32.h>
> >> > +#endif
> >> >  #include <linux/unaligned.h>
> >> >  #include <asm/bitsperlong.h>
> >> 
> >> Don't add #ifdef blocks around headers. If the header cannot
> >> be included without side-effects, change the linux/crc32.h
> >> file instead of its users.
> >
> > linux/acpi.h does that like many othes. What exactly is wrong with
> > protecting headers inclusion?
> 
> There is no "protecting" here, you just add complexity to the
> build when headers are sometimes included indirectly and but
> other times are not, depending on kernel configuration.

Sorry, don't understand... I use the 'protecting' term with the meaning:
the functionality that is explicitly disabled should be never used.
Otherwise, what for we disable it?

This comes with the cost of increasing the config space, but that's
how it works.

In terms of build complexity, I guess that the more you disable on
preprocessing stage, the less a compiler should actually process.

> It's unlikely to cause problems for the crc32.h header, but
> the acpi example definitely risks running into circular
> inclusions when you end up with some other header that depending
> on configuration ends up including linux/acpi.h while also
> bring included indirectly from that one.
> 
> >> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
> >> in include/asm-generic/bitops/__bitrev.h, which ends up
> >> hinding the generic___bitrev32() helper without need.
> >> 
> >> Simply removing the #ifdef there should avoid the build failure.
> >
> > OK, it seems like this is what I don't understand.
> >
> > We've got an optional feature, like CRC32, which is enabled by
> > CONFIG_CRC32. The most conservative way is to declare everything
> > CRC32-related in the corresponding header, and then protect the header
> > with IS_ENABLED(CONFIG_CRC32).
> >
> > I understand that from practical perspective, we can declare some simple
> > macros, like header size, unprotected. But what we've got now is a sort
> > of mess: all CRC32-related functions are declared unprotected, and
> > generic headers are good to use them. Compiler is happy while those
> > functions are actually unused. Next, CRC32 depends on BITREVERSE, which
> > is again unprotected, and it may optionally have an arch implementation.
> >
> > So if arch bitrev() is implemented, you can use part of bitreverse and
> > crc32 APIs despite that they are explicitly disabled - just because they
> > are implemented as macros in unprotected headers. And you cannot use some
> > others - because they are implemented differently, as a real functions.
> 
> I think you trying to solve a non-problem here.

This was reported by Nathan for tinyconfig. At least x86 and s390 are
affected.

https://lore.kernel.org/all/20260429202922.GA3575295@ax162/

Is tinyconfig important?

> It is extremely rare
> for any kernel to be built without crc32 or bitrev. Even in randconfig
> builds, both are usually selected by some driver. The behavior
> we've always had here is that in the rare one randconfig in 10000
> that hits the one driver missing 'select BITREVERSE', we get a link
> failure with an obvious fix. If you add the extra #ifdef, it gets
> a little more likely to actually run into the bug, and it shows
> up a little earlier as a compile failure instead of a link failure,
> but otherwise, nothing changes.

Except that it fixes the tinyconfig build.

The above argument sounds to me like you believe that CRC32 config is
really useless because it's enabled for any practical configuration.
And the same for BITREVERSE. Would it be better to get rid of them?
Would simplification of config space overweights the bloat of the
tinyconfig?

On the context: CONFIG_CRC32 is pre-historic, and BITREVERSE is 2006.

Right now half CRC32 is available if CONFIG_CRC32 is on, and half is
not available. The bitreverse is the same. If HAVE_ARCH_BITREVERSE is
enabled, one can use the API, bypassing the BITREVERSE. This doesn't
sound right to me long-term.

Whatever this ends up, let's figure out a consistent solution please?

Thanks,
Yury

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Arnd Bergmann]

On Mon, May 4, 2026, at 20:32, Yury Norov wrote:
> On Mon, May 04, 2026 at 07:18:49PM +0200, Arnd Bergmann wrote:
>> On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
>> > Never heard about such a thing like "optional interface". And git grep
>> > tends to second that...
>> 
>> I meant any library interface that can be turned on or off
>
> So? If I disable CRC32, can I use the either_crc()? In case of that
> networking header, the answer is yes. In some other piece of code
> the answer is no. Is that correct?

Since it's a macro defiend in terms of both bitref32 and
crc32_le, you can only call it from dead code, such as an
inline function that is not itself used, or from inside of
a block that is protected with IS_ENABLED(CONFIG_CRC32) etc.
  
>> >> 
>> >> Don't add #ifdef blocks around headers. If the header cannot
>> >> be included without side-effects, change the linux/crc32.h
>> >> file instead of its users.
>> >
>> > linux/acpi.h does that like many othes. What exactly is wrong with
>> > protecting headers inclusion?
>> 
>> There is no "protecting" here, you just add complexity to the
>> build when headers are sometimes included indirectly and but
>> other times are not, depending on kernel configuration.
>
> Sorry, don't understand... I use the 'protecting' term with the meaning:
> the functionality that is explicitly disabled should be never used.
> Otherwise, what for we disable it?

Arguably, both configuration symbols are at the point of not actually
saving enough object code size to actually be worth the Kconfig
dependencies.

As long as we have CONFIG_CRC32 and CONFIG_BITREVERSE, the
point of having the Kconfig symbols is to let drivers request
the inclusion of the library helpers.

>> It's unlikely to cause problems for the crc32.h header, but
>> the acpi example definitely risks running into circular
>> inclusions when you end up with some other header that depending
>> on configuration ends up including linux/acpi.h while also
>> bring included indirectly from that one.
>> 
>> >> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
>> >> in include/asm-generic/bitops/__bitrev.h, which ends up
>> >> hinding the generic___bitrev32() helper without need.
>> >> 
>> >> Simply removing the #ifdef there should avoid the build failure.
>> >
>> > OK, it seems like this is what I don't understand.
>> >
>> > We've got an optional feature, like CRC32, which is enabled by
>> > CONFIG_CRC32. The most conservative way is to declare everything
>> > CRC32-related in the corresponding header, and then protect the header
>> > with IS_ENABLED(CONFIG_CRC32).
>> >
>> > I understand that from practical perspective, we can declare some simple
>> > macros, like header size, unprotected. But what we've got now is a sort
>> > of mess: all CRC32-related functions are declared unprotected, and
>> > generic headers are good to use them. Compiler is happy while those
>> > functions are actually unused. Next, CRC32 depends on BITREVERSE, which
>> > is again unprotected, and it may optionally have an arch implementation.
>> >
>> > So if arch bitrev() is implemented, you can use part of bitreverse and
>> > crc32 APIs despite that they are explicitly disabled - just because they
>> > are implemented as macros in unprotected headers. And you cannot use some
>> > others - because they are implemented differently, as a real functions.
>> 
>> I think you trying to solve a non-problem here.
>
> This was reported by Nathan for tinyconfig. At least x86 and s390 are
> affected.
>
> https://lore.kernel.org/all/20260429202922.GA3575295@ax162/
>
> Is tinyconfig important?

Nathan reported a build regression caused by a small mistake
in 596a9ea9015b ("bitops: Define generic __bitrev8/16/32 for reuse"),
which is of course needs to be fixed.

What I meant is that there is no reason to not use the obvious
fix and do

--- a/include/asm-generic/bitops/__bitrev.h
+++ b/include/asm-generic/bitops/__bitrev.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_GENERIC_BITOPS___BITREV_H_
 #define _ASM_GENERIC_BITOPS___BITREV_H_
 
-#ifdef CONFIG_GENERIC_BITREVERSE
 #include <asm/types.h>
 
 extern u8 const byte_rev_table[256];
@@ -20,6 +19,5 @@ static __always_inline __attribute_const__ u32 generic___bitrev32(u32 x)
 {
        return (generic___bitrev16(x & 0xffff) << 16) | generic___bitrev16(x >> 16);
 }
-#endif /* CONFIG_GENERIC_BITREVERSE */
 
 #endif /* _ASM_GENERIC_BITOPS___BITREV_H_ */

> Right now half CRC32 is available if CONFIG_CRC32 is on, and half is
> not available. The bitreverse is the same. If HAVE_ARCH_BITREVERSE is
> enabled, one can use the API, bypassing the BITREVERSE. This doesn't
> sound right to me long-term.
>
> Whatever this ends up, let's figure out a consistent solution please?

I really don't think we need any sort of solution here, aside from
the trivial regression fix that returns it to the previous working
state.

      Arnd

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Yury Norov]

On Mon, May 04, 2026 at 09:05:30PM +0200, Arnd Bergmann wrote:
> On Mon, May 4, 2026, at 20:32, Yury Norov wrote:
> > On Mon, May 04, 2026 at 07:18:49PM +0200, Arnd Bergmann wrote:
> >> On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
> >> > Never heard about such a thing like "optional interface". And git grep
> >> > tends to second that...
> >> 
> >> I meant any library interface that can be turned on or off
> >
> > So? If I disable CRC32, can I use the either_crc()? In case of that
> > networking header, the answer is yes. In some other piece of code
> > the answer is no. Is that correct?
> 
> Since it's a macro defiend in terms of both bitref32 and
> crc32_le, you can only call it from dead code, such as an
> inline function that is not itself used, or from inside of
> a block that is protected with IS_ENABLED(CONFIG_CRC32) etc.
>   
> >> >> 
> >> >> Don't add #ifdef blocks around headers. If the header cannot
> >> >> be included without side-effects, change the linux/crc32.h
> >> >> file instead of its users.
> >> >
> >> > linux/acpi.h does that like many othes. What exactly is wrong with
> >> > protecting headers inclusion?
> >> 
> >> There is no "protecting" here, you just add complexity to the
> >> build when headers are sometimes included indirectly and but
> >> other times are not, depending on kernel configuration.
> >
> > Sorry, don't understand... I use the 'protecting' term with the meaning:
> > the functionality that is explicitly disabled should be never used.
> > Otherwise, what for we disable it?
> 
> Arguably, both configuration symbols are at the point of not actually
> saving enough object code size to actually be worth the Kconfig
> dependencies.
> 
> As long as we have CONFIG_CRC32 and CONFIG_BITREVERSE, the
> point of having the Kconfig symbols is to let drivers request
> the inclusion of the library helpers.
> 
> >> It's unlikely to cause problems for the crc32.h header, but
> >> the acpi example definitely risks running into circular
> >> inclusions when you end up with some other header that depending
> >> on configuration ends up including linux/acpi.h while also
> >> bring included indirectly from that one.
> >> 
> >> >> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
> >> >> in include/asm-generic/bitops/__bitrev.h, which ends up
> >> >> hinding the generic___bitrev32() helper without need.
> >> >> 
> >> >> Simply removing the #ifdef there should avoid the build failure.
> >> >
> >> > OK, it seems like this is what I don't understand.
> >> >
> >> > We've got an optional feature, like CRC32, which is enabled by
> >> > CONFIG_CRC32. The most conservative way is to declare everything
> >> > CRC32-related in the corresponding header, and then protect the header
> >> > with IS_ENABLED(CONFIG_CRC32).
> >> >
> >> > I understand that from practical perspective, we can declare some simple
> >> > macros, like header size, unprotected. But what we've got now is a sort
> >> > of mess: all CRC32-related functions are declared unprotected, and
> >> > generic headers are good to use them. Compiler is happy while those
> >> > functions are actually unused. Next, CRC32 depends on BITREVERSE, which
> >> > is again unprotected, and it may optionally have an arch implementation.
> >> >
> >> > So if arch bitrev() is implemented, you can use part of bitreverse and
> >> > crc32 APIs despite that they are explicitly disabled - just because they
> >> > are implemented as macros in unprotected headers. And you cannot use some
> >> > others - because they are implemented differently, as a real functions.
> >> 
> >> I think you trying to solve a non-problem here.
> >
> > This was reported by Nathan for tinyconfig. At least x86 and s390 are
> > affected.
> >
> > https://lore.kernel.org/all/20260429202922.GA3575295@ax162/
> >
> > Is tinyconfig important?
> 
> Nathan reported a build regression caused by a small mistake
> in 596a9ea9015b ("bitops: Define generic __bitrev8/16/32 for reuse"),
> which is of course needs to be fixed.
> 
> What I meant is that there is no reason to not use the obvious
> fix and do
> 
> --- a/include/asm-generic/bitops/__bitrev.h
> +++ b/include/asm-generic/bitops/__bitrev.h
> @@ -2,7 +2,6 @@
>  #ifndef _ASM_GENERIC_BITOPS___BITREV_H_
>  #define _ASM_GENERIC_BITOPS___BITREV_H_
>  
> -#ifdef CONFIG_GENERIC_BITREVERSE
>  #include <asm/types.h>
>  
>  extern u8 const byte_rev_table[256];
> @@ -20,6 +19,5 @@ static __always_inline __attribute_const__ u32 generic___bitrev32(u32 x)
>  {
>         return (generic___bitrev16(x & 0xffff) << 16) | generic___bitrev16(x >> 16);
>  }
> -#endif /* CONFIG_GENERIC_BITREVERSE */
>  
>  #endif /* _ASM_GENERIC_BITOPS___BITREV_H_ */
> 
> > Right now half CRC32 is available if CONFIG_CRC32 is on, and half is
> > not available. The bitreverse is the same. If HAVE_ARCH_BITREVERSE is
> > enabled, one can use the API, bypassing the BITREVERSE. This doesn't
> > sound right to me long-term.
> >
> > Whatever this ends up, let's figure out a consistent solution please?
> 
> I really don't think we need any sort of solution here, aside from
> the trivial regression fix that returns it to the previous working
> state.

OK. Not that I've got the answers to my questions, but I trust your
expertise, so will do as suggested.

Thanks,
Yury

Re: [PATCH 1/6] lib: include crc32.h conditionally on CONFIG_CRC32

[Eric Biggers]

On Mon, May 04, 2026 at 09:05:30PM +0200, Arnd Bergmann wrote:
> On Mon, May 4, 2026, at 20:32, Yury Norov wrote:
> > On Mon, May 04, 2026 at 07:18:49PM +0200, Arnd Bergmann wrote:
> >> On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
> >> > Never heard about such a thing like "optional interface". And git grep
> >> > tends to second that...
> >> 
> >> I meant any library interface that can be turned on or off
> >
> > So? If I disable CRC32, can I use the either_crc()? In case of that
> > networking header, the answer is yes. In some other piece of code
> > the answer is no. Is that correct?
> 
> Since it's a macro defiend in terms of both bitref32 and
> crc32_le, you can only call it from dead code, such as an
> inline function that is not itself used, or from inside of
> a block that is protected with IS_ENABLED(CONFIG_CRC32) etc.
>   
> >> >> 
> >> >> Don't add #ifdef blocks around headers. If the header cannot
> >> >> be included without side-effects, change the linux/crc32.h
> >> >> file instead of its users.
> >> >
> >> > linux/acpi.h does that like many othes. What exactly is wrong with
> >> > protecting headers inclusion?
> >> 
> >> There is no "protecting" here, you just add complexity to the
> >> build when headers are sometimes included indirectly and but
> >> other times are not, depending on kernel configuration.
> >
> > Sorry, don't understand... I use the 'protecting' term with the meaning:
> > the functionality that is explicitly disabled should be never used.
> > Otherwise, what for we disable it?
> 
> Arguably, both configuration symbols are at the point of not actually
> saving enough object code size to actually be worth the Kconfig
> dependencies.
> 
> As long as we have CONFIG_CRC32 and CONFIG_BITREVERSE, the
> point of having the Kconfig symbols is to let drivers request
> the inclusion of the library helpers.
> 
> >> It's unlikely to cause problems for the crc32.h header, but
> >> the acpi example definitely risks running into circular
> >> inclusions when you end up with some other header that depending
> >> on configuration ends up including linux/acpi.h while also
> >> bring included indirectly from that one.
> >> 
> >> >> It looks like the problem is the check for CONFIG_GENERIC_BITREVERSE
> >> >> in include/asm-generic/bitops/__bitrev.h, which ends up
> >> >> hinding the generic___bitrev32() helper without need.
> >> >> 
> >> >> Simply removing the #ifdef there should avoid the build failure.
> >> >
> >> > OK, it seems like this is what I don't understand.
> >> >
> >> > We've got an optional feature, like CRC32, which is enabled by
> >> > CONFIG_CRC32. The most conservative way is to declare everything
> >> > CRC32-related in the corresponding header, and then protect the header
> >> > with IS_ENABLED(CONFIG_CRC32).
> >> >
> >> > I understand that from practical perspective, we can declare some simple
> >> > macros, like header size, unprotected. But what we've got now is a sort
> >> > of mess: all CRC32-related functions are declared unprotected, and
> >> > generic headers are good to use them. Compiler is happy while those
> >> > functions are actually unused. Next, CRC32 depends on BITREVERSE, which
> >> > is again unprotected, and it may optionally have an arch implementation.
> >> >
> >> > So if arch bitrev() is implemented, you can use part of bitreverse and
> >> > crc32 APIs despite that they are explicitly disabled - just because they
> >> > are implemented as macros in unprotected headers. And you cannot use some
> >> > others - because they are implemented differently, as a real functions.
> >> 
> >> I think you trying to solve a non-problem here.
> >
> > This was reported by Nathan for tinyconfig. At least x86 and s390 are
> > affected.
> >
> > https://lore.kernel.org/all/20260429202922.GA3575295@ax162/
> >
> > Is tinyconfig important?
> 
> Nathan reported a build regression caused by a small mistake
> in 596a9ea9015b ("bitops: Define generic __bitrev8/16/32 for reuse"),
> which is of course needs to be fixed.
> 
> What I meant is that there is no reason to not use the obvious
> fix and do
> 
> --- a/include/asm-generic/bitops/__bitrev.h
> +++ b/include/asm-generic/bitops/__bitrev.h
> @@ -2,7 +2,6 @@
>  #ifndef _ASM_GENERIC_BITOPS___BITREV_H_
>  #define _ASM_GENERIC_BITOPS___BITREV_H_
>  
> -#ifdef CONFIG_GENERIC_BITREVERSE
>  #include <asm/types.h>
>  
>  extern u8 const byte_rev_table[256];
> @@ -20,6 +19,5 @@ static __always_inline __attribute_const__ u32 generic___bitrev32(u32 x)
>  {
>         return (generic___bitrev16(x & 0xffff) << 16) | generic___bitrev16(x >> 16);
>  }
> -#endif /* CONFIG_GENERIC_BITREVERSE */
>  
>  #endif /* _ASM_GENERIC_BITOPS___BITREV_H_ */
> 
> > Right now half CRC32 is available if CONFIG_CRC32 is on, and half is
> > not available. The bitreverse is the same. If HAVE_ARCH_BITREVERSE is
> > enabled, one can use the API, bypassing the BITREVERSE. This doesn't
> > sound right to me long-term.
> >
> > Whatever this ends up, let's figure out a consistent solution please?
> 
> I really don't think we need any sort of solution here, aside from
> the trivial regression fix that returns it to the previous working
> state.

I agree with Arnd.  The standard practice in the kernel is for headers
to be includeable even when the associated kconfig (if any) isn't
enabled.  In such a case usually the inline functions, macros, and the
declarations of out-of-line functions and global variables are still
made available in the header.

The point of the kconfig symbol is just to build the file containing the
definitions of out-of-line functions and global variables.

Yes, when those definitions are needed and calling code forgets to
select that kconfig symbol, a build error results.  But it's "only" a
build error; those get found and fixed relatively easily.  The
alternative of hiding everything would mean a lot more #ifdefs in other
code, which ends up being very messy.

- Eric

[PATCH 2/6] lib/bitrev: Introduce GENERIC_BITREVERSE and cleanup Kconfig

[Yury Norov]

From: Jinjie Ruan <ruanjinjie@huawei.com>

Currently, the bit reversal lookup table is controlled by
!HAVE_ARCH_BITREVERSE. This makes it difficult for architectures to
provide a hardware-accelerated implementation while still falling
back to the generic table for specific configurations.

Introduce CONFIG_GENERIC_BITREVERSE to explicitly manage the generic
lookup table implementation. By using 'def_bool !HAVE_ARCH_BITREVERSE'
with a dependency on 'BITREVERSE', we ensure that:

1. The table is only compiled when needed.
2. The .config is not polluted with useless options when BITREVERSE
   is disabled.
3. Avoids bloating the .data section for architectures that have
   full hardware bit-reverse support and don't need the table.

Update lib/bitrev.c to use CONFIG_GENERIC_BITREVERSE instead of
checking the absence of HAVE_ARCH_BITREVERSE. This provides a
cleaner interface for architectures like RISC-V that may want to
selectively use the generic implementation as a fallback.

Suggested-by: David Laight <David.Laight@ACULAB.COM>
Suggested-by: Yury Norov <ynorov@nvidia.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 lib/Kconfig  | 18 ++++++++++++++++++
 lib/bitrev.c |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 00a9509636c1..3ac12308eb76 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -62,6 +62,24 @@ config HAVE_ARCH_BITREVERSE
 	  This option enables the use of hardware bit-reversal instructions on
 	  architectures which support such operations.
 
+config GENERIC_BITREVERSE
+	def_bool !HAVE_ARCH_BITREVERSE
+	depends on BITREVERSE
+	help
+	  This option provides the standard software-based bit reversal
+	  implementation using a lookup table.
+
+	  Architecture-specific implementations (HAVE_ARCH_BITREVERSE)
+	  and this generic version are not necessarily mutually exclusive
+	  at the configuration level, but selecting this ensures that
+	  the generic `bitrev8/16/32` functions are available when the
+	  CPU does not provide native instructions (like RISC-V's ZBKB
+	  extension).
+
+	  If you are an architecture maintainer and your CPU has native
+	  bit-reversal instructions, you should select HAVE_ARCH_BITREVERSE
+	  to skip this table-based implementation.
+
 config ARCH_HAS_STRNCPY_FROM_USER
 	bool
 
diff --git a/lib/bitrev.c b/lib/bitrev.c
index 81b56e0a7f32..3a53ff67aeba 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
-#ifndef CONFIG_HAVE_ARCH_BITREVERSE
+#ifdef CONFIG_GENERIC_BITREVERSE
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/bitrev.h>
@@ -44,4 +44,4 @@ const u8 byte_rev_table[256] = {
 };
 EXPORT_SYMBOL_GPL(byte_rev_table);
 
-#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
+#endif /* CONFIG_GENERIC_BITREVERSE */
-- 
2.51.0

Re: [PATCH 2/6] lib/bitrev: Introduce GENERIC_BITREVERSE and cleanup Kconfig

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Build and linking failure for modules using bitreverse when CONFIG_BITREVERSE is compiled as a module ('m').
- [Medium] Kconfig unmet direct dependencies warning when GENERIC_BITREVERSE is selected by architectures.
--

commit 43d61911d4e52b31128989028f6541618dd3291b
Author: Jinjie Ruan <ruanjinjie@huawei.com>

lib/bitrev: Introduce GENERIC_BITREVERSE and cleanup Kconfig

This commit introduces the GENERIC_BITREVERSE config option to explicitly
manage the generic bit reversal lookup table, replacing the previous check for
!HAVE_ARCH_BITREVERSE. It updates lib/bitrev.c to use this new symbol and
makes it dependent on the BITREVERSE config.

> diff --git a/lib/Kconfig b/lib/Kconfig
> index 00a9509636c18..3ac12308eb765 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -62,6 +62,24 @@ config HAVE_ARCH_BITREVERSE
>  	  This option enables the use of hardware bit-reversal instructions on
>  	  architectures which support such operations.
>  
> +config GENERIC_BITREVERSE
> +	def_bool !HAVE_ARCH_BITREVERSE
> +	depends on BITREVERSE

Because BITREVERSE is a tristate symbol and GENERIC_BITREVERSE is defined as a
bool, what happens when BITREVERSE is configured as a module (=m)?

Kconfig evaluates a bool depending on a module as 'n'. This means when
CONFIG_BITREVERSE=m, CONFIG_GENERIC_BITREVERSE evaluates to 'n' and remains
undefined.

Will this result in missing declarations and linker failures for any modules
relying on the bitreverse library?

Additionally, does this strict dependency cause Kconfig unmet direct dependency
warnings if an architecture subsequently uses select GENERIC_BITREVERSE but
the kernel configuration has BITREVERSE disabled (such as in allnoconfig)?

> +	help
> +	  This option provides the standard software-based bit reversal

[ ... ]

> diff --git a/lib/bitrev.c b/lib/bitrev.c
> index 81b56e0a7f325..3a53ff67aebaa 100644
> --- a/lib/bitrev.c
> +++ b/lib/bitrev.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -#ifndef CONFIG_HAVE_ARCH_BITREVERSE
> +#ifdef CONFIG_GENERIC_BITREVERSE

If CONFIG_GENERIC_BITREVERSE evaluates to 'n' when CONFIG_BITREVERSE=m,
this file will be compiled empty. Does this prevent byte_rev_table from being
exported for modules that need it, causing a modpost linking failure?

>  #include <linux/types.h>
>  #include <linux/module.h>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430211351.658193-1-ynorov@nvidia.com?part=2

[PATCH 3/6] bitops: Define generic __bitrev8/16/32 for reuse

[Yury Norov]

From: Jinjie Ruan <ruanjinjie@huawei.com>

Define generic __bitrev8/16/32 using the implementation
in <linux/bitrev.h>, so they can be reused in <asm/bitrev.h>,
such as RISCV.

Reviewed-by: Yury Norov <ynorov@nvidia.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 include/asm-generic/bitops/__bitrev.h | 25 +++++++++++++++++++++++++
 include/linux/bitrev.h                | 20 ++++----------------
 2 files changed, 29 insertions(+), 16 deletions(-)
 create mode 100644 include/asm-generic/bitops/__bitrev.h

diff --git a/include/asm-generic/bitops/__bitrev.h b/include/asm-generic/bitops/__bitrev.h
new file mode 100644
index 000000000000..f06af929678d
--- /dev/null
+++ b/include/asm-generic/bitops/__bitrev.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_GENERIC_BITOPS___BITREV_H_
+#define _ASM_GENERIC_BITOPS___BITREV_H_
+
+#ifdef CONFIG_GENERIC_BITREVERSE
+#include <asm/types.h>
+
+extern u8 const byte_rev_table[256];
+static __always_inline __attribute_const__ u8 generic___bitrev8(u8 byte)
+{
+	return byte_rev_table[byte];
+}
+
+static __always_inline __attribute_const__ u16 generic___bitrev16(u16 x)
+{
+	return (generic___bitrev8(x & 0xff) << 8) | generic___bitrev8(x >> 8);
+}
+
+static __always_inline __attribute_const__ u32 generic___bitrev32(u32 x)
+{
+	return (generic___bitrev16(x & 0xffff) << 16) | generic___bitrev16(x >> 16);
+}
+#endif /* CONFIG_GENERIC_BITREVERSE */
+
+#endif /* _ASM_GENERIC_BITOPS___BITREV_H_ */
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index d35b8ec1c485..11620a70e776 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -12,22 +12,10 @@
 #define __bitrev8 __arch_bitrev8
 
 #else
-extern u8 const byte_rev_table[256];
-static inline u8 __bitrev8(u8 byte)
-{
-	return byte_rev_table[byte];
-}
-
-static inline u16 __bitrev16(u16 x)
-{
-	return (__bitrev8(x & 0xff) << 8) | __bitrev8(x >> 8);
-}
-
-static inline u32 __bitrev32(u32 x)
-{
-	return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
-}
-
+#include <asm-generic/bitops/__bitrev.h>
+#define __bitrev32 generic___bitrev32
+#define __bitrev16 generic___bitrev16
+#define __bitrev8 generic___bitrev8
 #endif /* CONFIG_HAVE_ARCH_BITREVERSE */
 
 #define __bitrev8x4(x)	(__bitrev32(swab32(x)))
-- 
2.51.0

Re: [PATCH 3/6] bitops: Define generic __bitrev8/16/32 for reuse

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The #ifdef CONFIG_GENERIC_BITREVERSE guard in <asm-generic/bitops/__bitrev.h> introduces build regressions (implicit function declarations) for modules and uninstantiated inline functions.
--

commit d29d1db3f8ae8fed09b1d7cc283d610df9bcb8b6
Author: Jinjie Ruan <ruanjinjie@huawei.com>

bitops: Define generic __bitrev8/16/32 for reuse

This commit extracts the generic bit reversal functions into a new
<asm-generic/bitops/__bitrev.h> header, allowing architectures to reuse them.

> diff --git a/include/asm-generic/bitops/__bitrev.h b/include/asm-generic/bitops/__bitrev.h
> new file mode 100644
> index 0000000000000..f06af929678d6
> --- /dev/null
> +++ b/include/asm-generic/bitops/__bitrev.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_GENERIC_BITOPS___BITREV_H_
> +#define _ASM_GENERIC_BITOPS___BITREV_H_
> +
> +#ifdef CONFIG_GENERIC_BITREVERSE

Will this cause build regressions for modules when CONFIG_BITREVERSE=m?

If CONFIG_GENERIC_BITREVERSE is a bool depending on CONFIG_BITREVERSE, when
CONFIG_BITREVERSE=m, the bool evaluates to n. This causes
CONFIG_GENERIC_BITREVERSE to be n, meaning the generic___bitrev* declarations
are removed. Any module calling bitrev8() would then fail to compile due to an
implicit function declaration.

This also means that compiling bitrev.o based on CONFIG_GENERIC_BITREVERSE
prevents it from being built as a module.

> +#include <asm/types.h>
> +
> +extern u8 const byte_rev_table[256];
> +static __always_inline __attribute_const__ u8 generic___bitrev8(u8 byte)

[ ... ]

> diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
> index d35b8ec1c485c..11620a70e7762 100644
> --- a/include/linux/bitrev.h
> +++ b/include/linux/bitrev.h
> @@ -12,22 +12,10 @@
>  #define __bitrev8 __arch_bitrev8
>  
>  #else
> -extern u8 const byte_rev_table[256];
> -static inline u8 __bitrev8(u8 byte)
> -{
> -	return byte_rev_table[byte];
> -}
> -
> -static inline u16 __bitrev16(u16 x)
> -{
> -	return (__bitrev8(x & 0xff) << 8) | __bitrev8(x >> 8);
> -}
> -
> -static inline u32 __bitrev32(u32 x)
> -{
> -	return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
> -}
> -
> +#include <asm-generic/bitops/__bitrev.h>
> +#define __bitrev32 generic___bitrev32
> +#define __bitrev16 generic___bitrev16
> +#define __bitrev8 generic___bitrev8

Does this introduce header fragility if CONFIG_BITREVERSE=n?

If CONFIG_BITREVERSE=n, the guard in <asm-generic/bitops/__bitrev.h> is
skipped, but __bitrev32 is still unconditionally mapped to generic___bitrev32
here.

Any C file including <linux/bitrev.h> that contains an uninstantiated static
inline function using bitrev32() will fail to compile with
-Werror=implicit-function-declaration. Previously, unconditional declarations
in <linux/bitrev.h> safely deferred such errors to link-time.

>  #endif /* CONFIG_HAVE_ARCH_BITREVERSE */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430211351.658193-1-ynorov@nvidia.com?part=3

[PATCH 4/6] arch/riscv: Add bitrev.h file to support rev8 and brev8

[Yury Norov]

From: Jinjie Ruan <ruanjinjie@huawei.com>

The RISC-V Bit-manipulation Extension for Cryptography (Zbkb) provides
the 'brev8' instruction, which reverses the bits within each byte.
Combined with the 'rev8' instruction (from Zbb or Zbkb), which reverses
the byte order of a register, we can efficiently implement 16-bit,
32-bit, and (on RV64) 64-bit bit reversal.

This is significantly faster than the default software table-lookup
implementation in lib/bitrev.c, as it replaces memory accesses and
multiple arithmetic operations with just two or three hardware
instructions.

Select HAVE_ARCH_BITREVERSE as well as GENERIC_BITREVERSE,
and provide <asm/bitrev.h> to utilize these instructions when
the Zbkb extension is available at runtime via the alternatives
mechanism.

Link: https://docs.riscv.org/reference/isa/unpriv/b-st-ext.html
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 arch/riscv/Kconfig              |  2 ++
 arch/riscv/include/asm/bitrev.h | 51 +++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 arch/riscv/include/asm/bitrev.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d235396c4514..d32309846fa3 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -104,6 +104,7 @@ config RISCV
 	select FUNCTION_ALIGNMENT_8B if DYNAMIC_FTRACE_WITH_CALL_OPS
 	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_ATOMIC64 if !64BIT
+	select GENERIC_BITREVERSE
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select GENERIC_CPU_DEVICES
 	select GENERIC_CPU_VULNERABILITIES
@@ -128,6 +129,7 @@ config RISCV
 	select HAS_IOPORT if MMU
 	select HAVE_ALIGNED_STRUCT_PAGE
 	select HAVE_ARCH_AUDITSYSCALL
+	select HAVE_ARCH_BITREVERSE if RISCV_ISA_ZBKB
 	select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
 	select HAVE_ARCH_HUGE_VMAP if MMU && 64BIT
 	select HAVE_ARCH_JUMP_LABEL
diff --git a/arch/riscv/include/asm/bitrev.h b/arch/riscv/include/asm/bitrev.h
new file mode 100644
index 000000000000..4b9b8d34cc3b
--- /dev/null
+++ b/arch/riscv/include/asm/bitrev.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_BITREV_H
+#define __ASM_BITREV_H
+
+#include <linux/types.h>
+#include <asm/cpufeature-macros.h>
+#include <asm/hwcap.h>
+#include <asm-generic/bitops/__bitrev.h>
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+	unsigned long result;
+
+	if (!riscv_has_extension_likely(RISCV_ISA_EXT_ZBKB))
+		return generic___bitrev32(x);
+
+	asm volatile(
+		".option push\n"
+		".option arch,+zbkb\n"
+		"rev8 %0, %1\n"
+		"brev8 %0, %0\n"
+		".option pop"
+		: "=r" (result) : "r" ((long)x)
+	);
+
+	return result >> (__riscv_xlen - 32);
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+	return __arch_bitrev32(x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+	unsigned long result;
+
+	if (!riscv_has_extension_likely(RISCV_ISA_EXT_ZBKB))
+		return generic___bitrev8(x);
+
+	asm volatile(
+		".option push\n"
+		".option arch,+zbkb\n"
+		"brev8 %0, %1\n"
+		".option pop"
+		: "=r" (result) : "r" ((long)x)
+	);
+
+	return result;
+}
+#endif
-- 
2.51.0

Re: [PATCH 4/6] arch/riscv: Add bitrev.h file to support rev8 and brev8

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Selecting GENERIC_BITREVERSE without satisfying its dependency causes a Kconfig unmet dependency warning.
--

commit b26322371528c694cf7ce4bf22b509fc4ef43133
Author: Jinjie Ruan <ruanjinjie@huawei.com>

arch/riscv: Add bitrev.h file to support rev8 and brev8

This commit adds <asm/bitrev.h> to efficiently implement bit reversal
using the 'rev8' and 'brev8' instructions from the RISC-V Zbkb extension.
It updates Kconfig to select GENERIC_BITREVERSE and HAVE_ARCH_BITREVERSE
to utilize these hardware instructions.

Link: https://docs.riscv.org/reference/isa/unpriv/b-st-ext.html

> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d235396c45141..d32309846fa31 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -104,6 +104,7 @@ config RISCV
>  	select FUNCTION_ALIGNMENT_8B if DYNAMIC_FTRACE_WITH_CALL_OPS
>  	select GENERIC_ARCH_TOPOLOGY
>  	select GENERIC_ATOMIC64 if !64BIT
> +	select GENERIC_BITREVERSE

Will this select introduce a Kconfig unmet dependency regression?

In lib/Kconfig, GENERIC_BITREVERSE explicitly depends on BITREVERSE.

Selecting a symbol directly without also satisfying its dependencies forces
the target symbol on, which causes Kconfig warnings at build time when
BITREVERSE is disabled (for example, during allnoconfig builds).

Could this select also include BITREVERSE to satisfy the dependency chain?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430211351.658193-1-ynorov@nvidia.com?part=4

[PATCH 5/6] lib: compile generic bitrev.c conditionally on GENERIC_BITREVERSE

[Yury Norov]

The file is compiled based on CONFIG_BITREVERSE=y, but everything inside
is protected with CONFIG_GENERIC_BITREVERSE.

Make it simpler by switching the Makefile to compile lib/bitrev.c based
on the proper config.

Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 lib/Makefile | 2 +-
 lib/bitrev.c | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index f33a24bf1c19..23e07d19d01c 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -145,7 +145,7 @@ obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
 obj-$(CONFIG_LIST_HARDENED) += list_debug.o
 obj-$(CONFIG_DEBUG_OBJECTS) += debugobjects.o
 
-obj-$(CONFIG_BITREVERSE) += bitrev.o
+obj-$(CONFIG_GENERIC_BITREVERSE) += bitrev.o
 obj-$(CONFIG_LINEAR_RANGES) += linear_ranges.o
 obj-$(CONFIG_PACKING)	+= packing.o
 obj-$(CONFIG_PACKING_KUNIT_TEST) += packing_test.o
diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3a53ff67aeba..05088231f31f 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0-only
-#ifdef CONFIG_GENERIC_BITREVERSE
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/bitrev.h>
@@ -43,5 +42,3 @@ const u8 byte_rev_table[256] = {
 	0x1f, 0x9f, 0x5f, 0xdf, 0x3f, 0xbf, 0x7f, 0xff,
 };
 EXPORT_SYMBOL_GPL(byte_rev_table);
-
-#endif /* CONFIG_GENERIC_BITREVERSE */
-- 
2.51.0

Re: [PATCH 5/6] lib: compile generic bitrev.c conditionally on GENERIC_BITREVERSE

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit breaks loadable module support for `lib/bitrev.c` by forcibly compiling it statically into the core kernel (`vmlinux`) even when configured as a module (`CONFIG_BITREVERSE=m`).
--

commit 7ea6ceee0bfd2cae761187221063672a5947f4c4
Author: Yury Norov <ynorov@nvidia.com>

lib: compile generic bitrev.c conditionally on GENERIC_BITREVERSE

This commit simplifies the build logic for the generic bit-reversal library.
It changes the Makefile to compile lib/bitrev.c based on
CONFIG_GENERIC_BITREVERSE instead of CONFIG_BITREVERSE, and removes the
corresponding preprocessor conditionals from the source file.

> diff --git a/lib/Makefile b/lib/Makefile
> index f33a24bf1c19a..23e07d19d01c9 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -145,7 +145,7 @@ obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
>  obj-$(CONFIG_LIST_HARDENED) += list_debug.o
>  obj-$(CONFIG_DEBUG_OBJECTS) += debugobjects.o
>  
> -obj-$(CONFIG_BITREVERSE) += bitrev.o
> +obj-$(CONFIG_GENERIC_BITREVERSE) += bitrev.o

Will this break loadable module support for lib/bitrev.c?

CONFIG_BITREVERSE is a tristate, allowing the library to be built as a
module. However, CONFIG_GENERIC_BITREVERSE is a boolean. When
CONFIG_BITREVERSE is set to modular, CONFIG_GENERIC_BITREVERSE evaluates
to y.

By switching the Makefile rule to use CONFIG_GENERIC_BITREVERSE, bitrev.o
will be built into the core kernel unconditionally when the generic
implementation is used, even if a modular build was requested.

Could this regression lead to unintended kernel bloat by ignoring the
configuration intent?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430211351.658193-1-ynorov@nvidia.com?part=5

[PATCH 6/6] MAINTAINERS: BITOPS: include bitrev.[ch]

[Yury Norov]

Arch bitrev API is covered in MAINTAINERS under the BITOPS entry,
while generic bitrev is unmaintained. Move it under BITOPS too.

Signed-off-by: Yury Norov <ynorov@nvidia.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 27a073f53cea..b69db2a7031f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4549,7 +4549,9 @@ F:	arch/*/lib/bitops.c
 F:	include/asm-generic/bitops
 F:	include/asm-generic/bitops.h
 F:	include/linux/bitops.h
+F:	include/linux/bitrev.h
 F:	include/linux/count_zeros.h
+F:	lib/bitrev.c
 F:	lib/hweight.c
 F:	lib/test_bitops.c
 F:	lib/tests/bitops_kunit.c
-- 
2.51.0

Re: [PATCH 0/6] lib: rework bitreverse

[Yury Norov]

On Thu, Apr 30, 2026 at 05:13:44PM -0400, Yury Norov wrote:
> This series is a resend for Jinjie Ruan's "arch/riscv: Add bitrev.h file
> to support rev8 and brev8" [1], my follow-up "lib: compile generic
> bitrev based on GENERIC_BITREVERSE" [2], and the fix for a build error
> reported by Nathan Chancellor [3].
> 
> No changes, except for combining pieces together and rebasing on top of
> the tree.
> 
> [1] https://lore.kernel.org/all/20260421130752.607500-1-ruanjinjie@huawei.com/
> [2] https://lore.kernel.org/all/20260427205210.397471-1-ynorov@nvidia.com/
> [3] https://lore.kernel.org/all/20260429202922.GA3575295@ax162/
> 
> Build-tested against x86 tinyconfig and defconfig, having disabled and
> enabiled CRC32 and BITREVERSE, correspondingly.

I've got a feedback from sashiko bot.

1. We need to do #if IS_DEFINED(CONFIG_CRC32) instead of #ifdef,
   because it may be a module.
2. Selecting GENERIC_BITREVERSE without BITREVERSE causes the unmet
   direct dependency warning:

   WARNING: unmet direct dependencies detected for GENERIC_BITREVERSE
  Depends on [n]: BITREVERSE [=n]
  Selected by [m]:
  - MYCONFIG [=m]

I'll send a v2 to fix it

Thanks,
Yury