Fix LZ4 endianness autodetection #2264
Conversation
Endianness detection in LZ4 is broken in user-space builds. This bug corrupts compressed data and manifests itself in several ztest failures. When LZ4 was originally ported to Illumos ZFS, the proper checks for Linux were stripped out. The Linux port then inherited the remaining detection code that works on Illumos but not on Linux. The current LZ4 endianness check misuses the condition defined(__BIG_ENDIAN) to indicate a big-endian system. On Linux __BIG_ENDIAN is defined uncondtionally in the user-space header /usr/include/endian.h, regardless of the endianness of the system. The kernel does not use this header, so only user-space builds are affected. While we could fix this by restoring the upstream LZ4 endianness detection code, reliable checks already exist in libspl/include/sys/isa_defs.h. This change uses the libspl results to replace the word-size and endianness checks in LZ4, simplifying the code and reducing duplication. Fixes openzfs#1963 Fixes openzfs#1964 Fixes openzfs#1965 Signed-off-by: Ned Bass <bass6@llnl.gov>
|
Thanks Ned this looks like a nice simplification. Early test results look good but I'll let ztest run all weekend just to make sure. This issue neatly explains all three test issues you mentioned in the commit message. |
|
Looks okay to me. I'm running my own modified ztest along with this patch and not getting any problems. The #undef strikes me as superfluous but that's no big deal. Signed-off-by: DHE git@dehacked.net |
|
I ran ztest for 8 hours last night with this patch and saw no failures! Nice! |
|
This looks good, but as @DeHackEd said, I don't think #undef is necessary. |
|
The #undef isn't needed, but I added it as a form of documentation. I want to make it explicit that we're using the defined state of LZ4_BIG_ENDIAN to indicate big-endianess, as opposed to a boolean 0 or 1 value. Especially since it is inconsistent with LZ4_ARCH64 above. |
|
Thanks everybody, this has been merged to master. de39ec1 Fix LZ4 endianness autodetection |
Endianness detection in LZ4 is broken in user-space builds. This
bug corrupts compressed data and manifests itself in several ztest
failures. When LZ4 was originally ported to Illumos ZFS, the proper
checks for Linux were stripped out. The Linux port then inherited
the remaining detection code that works on Illumos but not on Linux.
The current LZ4 endianness check misuses the condition
defined(__BIG_ENDIAN) to indicate a big-endian system. On Linux
__BIG_ENDIAN is defined uncondtionally in the user-space header
/usr/include/endian.h, regardless of the endianness of the system.
The kernel does not use this header, so only user-space builds are
affected.
While we could fix this by restoring the upstream LZ4 endianness
detection code, reliable checks already exist in
libspl/include/sys/isa_defs.h. This change uses the libspl results
to replace the word-size and endianness checks in LZ4, simplifying
the code and reducing duplication.
Fixes #1963
Fixes #1964
Fixes #1965
Signed-off-by: Ned Bass bass6@llnl.gov