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

[sashiko-bot@kernel.org]

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