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

[Yury Norov <ynorov@nvidia.com>]

On Wed, Apr 29, 2026 at 08:59:43PM -0700, Nathan Chancellor wrote:
> On Wed, Apr 29, 2026 at 09:47:12PM -0400, Yury Norov wrote:
> > + networking maintainers
> > 
> > On Wed, Apr 29, 2026 at 01:29:22PM -0700, Nathan Chancellor wrote:
> > > Hi Jinjie,
> > > 
> > > On Tue, Apr 21, 2026 at 09:07:51PM +0800, Jinjie Ruan wrote:
> > > > 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>
> > > > ---
> > > >  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
> > > 
> > > The dependencies of this symbol seem insufficient, as I can trigger a
> > > build failure on next-20260429 like so:
> > > 
> > >   $ 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
> > >   ...
> > > 
> > >   $ scripts/config -s BITREVERSE
> > >   undef
> > > 
> > >   $ rg BITREVERSE .config
> > 
> > Confirm the same for x86 tinyconfig.
> > 
> > The problem is that the patch makes generic bitrevXX() conditional
> > on CONFIG_GENERIC_BITREVERSE, while before they were conditional on
> > !CONFIG_HAVE_ARCH_BITREVERSE. So if you don't have arch bitreverse(),
> > and dont' enable BITREVERSE, the generic implementation is not defined
> > now.
> > 
> > Luckily, the only user of bitrev() in unconditionally compiled objects
> > is CRC32, and it's not needed if CRC32 is disabled.
> > 
> > This is the minimal working fix for me. Please let me know what do you
> > think. I can prepend Jinjie's pathes in my tree with it if it's OK for
> > you.
> 
> I will run it through my full build matrix overnight and report back
> with any problems noticed.
> 
> > diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> > index df8f88f63a70..245b206dd38b 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_CRCC32
> 
> Initial testing shows that this should be
> 
>   #ifdef CONFIG_CRC32
> 
> here and below though.

Sure. Please let me know if this works for you. If it does, I'll make
it a regular patch, combine with Jinjie's work and my follow-up and
resend. Meanwhile, removed all bitreverse material from
bitmap-for-next.

Thanks,
Yury