Re: [PATCH v2 1/5] arch: select HAVE_ARCH_BITREVERSE conditionally on BITREVERSE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit introduces a performance regression in fbdev by making the hardware capability flag HAVE_ARCH_BITREVERSE dependent on the BITREVERSE API, based on a factually incorrect premise.
--

commit 62f45d1f26fdd43913871c56d8d0139e89cc923a
Author: Yury Norov <ynorov@nvidia.com>

arch: select HAVE_ARCH_BITREVERSE conditionally on BITREVERSE

This commit modifies architecture Kconfig files to only select
HAVE_ARCH_BITREVERSE if BITREVERSE is enabled. The stated goal is to avoid
generating unneeded code on architectures that have bit reversal instructions
but do not currently need the API.

The commit message states that selecting the option when the API is not
needed "may lead to generating the unneeded code." Is this description
accurate?

Looking at the implementation, HAVE_ARCH_BITREVERSE only enables inline
assembly macros in linux/bitrev.h. The software fallback table in
lib/bitrev.c is strictly gated by obj-$(CONFIG_BITREVERSE) += bitrev.o in
lib/Makefile, so no out-of-line code is generated when BITREVERSE=n.

> diff --git a/lib/Kconfig b/lib/Kconfig
> index 00a9509636c18..d8e7e89ae3205 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -58,6 +58,7 @@ config BITREVERSE
>  config HAVE_ARCH_BITREVERSE
>  	bool
>  	default n
> +	depends on BITREVERSE
>  	help
>  	  This option enables the use of hardware bit-reversal instructions on
>  	  architectures which support such operations.

Does tying a hardware capability flag (HAVE_ARCH_BITREVERSE) to a software
API (BITREVERSE) cause unintended performance regressions elsewhere?

For example, drivers/video/fbdev/core/fb_draw.h opportunistically checks
for the hardware capability:

#if defined(CONFIG_HAVE_ARCH_BITREVERSE)

The fbdev core intentionally avoids selecting BITREVERSE to prevent pulling
in the 256-byte lookup table, but uses the fast, native hardware
instructions if the architecture supports them.

With this patch applied, if a user builds a kernel where BITREVERSE=n,
fb_draw.h will believe the architecture lacks hardware support. Won't this
force fbdev to fall back to the much slower manual bit-twiddling function
fb_comp() in its tight pixel-drawing loops?

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