Thread (16 messages) 16 messages, 4 authors, 2026-05-06

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

From: "Arnd Bergmann" <arnd@arndb.de>
Date: 2026-05-04 17:19:48
Also in: bpf, linux-arch, linux-riscv, lkml

On Mon, May 4, 2026, at 18:46, Yury Norov wrote:
On Mon, May 04, 2026 at 10:03:10AM +0200, Arnd Bergmann wrote:
quoted
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
 
quoted
quoted
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.
...
quoted
 #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.
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. 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help