On Mon, May 4, 2026, at 20:32, Yury Norov wrote:

On Mon, May 04, 2026 at 07:18:49PM +0200, Arnd Bergmann wrote:

quoted

On Mon, May 4, 2026, at 18:46, Yury Norov wrote:

quoted

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.
  

quoted

quoted

quoted

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.

quoted

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.

quoted

quoted

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/ (local)

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