On Fri, Jan 8, 2021 at 12:53 AM Darrick J. Wong [off-list ref] wrote:

On Thu, Jan 07, 2021 at 02:27:51PM -0800, Eric Biggers wrote:

quoted

On Thu, Jan 07, 2021 at 10:48:05PM +0100, Arnd Bergmann wrote:

quoted

On Thu, Jan 7, 2021 at 5:27 PM Theodore Ts'o [off-list ref] wrote:

quoted

On Thu, Jan 07, 2021 at 01:37:47PM +0000, Russell King - ARM Linux admin wrote:

quoted

quoted

The gcc bugzilla mentions backports into gcc-linaro, but I do not see
them in my git history.

So, do we raise the minimum gcc version for the kernel as a whole to 5.1
or just for aarch64?

Russell, Arnd, thanks so much for tracking down the root cause of the
bug!

There is one more thing that I wondered about when looking through
the ext4 code: Should it just call the crc32c_le() function directly
instead of going through the crypto layer? It seems that with Ard's
rework from 2018, that can just call the underlying architecture specific
implementation anyway.

It looks like that would work, although note that crc32c_le() uses the shash API
too, so it isn't any more "direct" than what ext4 does now.

Yes.

Ah, I see. I had only noticed the architecture specific overrides for
__crc32c_le(),
and the global __weak crc32_le() function in lib/crc32.c, but failed to notice
the crc32c_le() macro that redirects to crc32c().

quoted

Also, a potential issue is that the implementation of crc32c that crc32c_le()
uses might be chosen too early if the architecture-specific implementation of
crc32c is compiled as a module (e.g. crc32c-intel.ko).

This was the primary reason I chose to do it this way for ext4.

The other is that ext4 didn't use crc32c before metadata_csum, so
there's no point in pulling in the crypto layer if you're only going to
use older ext2 or ext3 filesystems.  That was 2010, maybe people have
stopped doing that?

The per-architecture overrides for __crc32c_le() are from 2018. With that
it should be possible to just always have the fastest implementation
(forcing them to be built-in normally), but not all architectures do this.

quoted

There are two ways this
could be fixed -- either by making it a proper library API like blake2s() that
can call the architecture-specific code directly, or by reconfiguring things
when a new crypto module is loaded (like what lib/crc-t10dif.c does).

Though I would like to see the library functions gain the ability to use
whatever is the fastest mechanism available once we can be reasonably
certain that all the platform-specific drivers have been loaded.

That said, IIRC most distros compile all of them into their
(increasingly large) vmlinuz files so maybe this isn't much of practical
concern?

I recently made checked the missing dependencies of drivers that
fail to 'select CRC32' but do call it directly. With those added, there
are now around 200 drivers that include it, and in practice you would
hardly find any kernel that doesn't have it built-in already. Most notably,
jbd2 already calls crc32_be(), so it is impossible to build an EXT4
without it. For memory-constrained embedded devices, it would probably
be more valuable to build without the crypto layer than without crc32.

       Arnd