-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
common: Leverage a better CRC32C implementation #57271
base: main
Are you sure you want to change the base?
Conversation
ISA-L provides a few different CRC32C implementations, of which Ceph has only ever linked against one (crc32_iscsi_00). The second implementation of CRC32C provided by ISA-L (crc32_iscsi_01) improves upon the first as it is used by Ceph in a couple of ways: 1) crc32_iscsi_01 explicitly handles and checks for < 8 byte buffers and computes the CRC32C value using the hardware-accelerated CRC32 instruction. In comparison, crc32_iscsi_00 prefetches too far in cases of small buffers, requiring the Ceph code to explicitly check and handle this case differently in software. This software-fallback implementation of CRC32 also comes with a different set of LUTs (look up tables) and is less efficient as it does not make use of the CRC32 instruction. 2) crc32_iscsi_00 makes use of large LUTs (look up tables) to effectively perform the modular reduction required to produce the CRC32C value. In constrast, crc32_iscsi_01 uses the PCLMUL instruction set to perform reductions 128-bits at a time with smaller LUTs, resulting in greater throughput and less data cache pollution. Fixes: https://tracker.ceph.com/issues/65791 Signed-off-by: Tyler Stachecki <tstachecki@bloomberg.net>
Aside -- I'm not sure the check for overread in
At which point this change was swept up: intel/isa-l@105eeb9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
jenkins retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though I don't know if there are any corner cases we should watch out for. Probably good to get this in testing early in the release cycle.
@tj90241 Does this get executed in a normal suite run? If so, lgtm. |
Hi @athanatos - any kind of cluster or use case will make extremely heavy use of this change as long as the hosts under QA have PCLMUL support (check |
[sjust@smithi105 ~]$ grep -I pclmul /proc/cpuinfo I see pclmulqdq in cpuinfo -- good enough? |
Technically you also need If you wanted to be extra pedantic, you can also validate the symbol is hit at runtime with e.g. (you could also set a breakpoint in |
@tj90241 I just want to ensure that we're getting test coverage with our normal integration tests. |
ISA-L provides a few different CRC32C implementations, of
which Ceph has only ever linked against one
(crc32_iscsi_00).
The second implementation of CRC32C provided by ISA-L
(crc32_iscsi_01) improves upon the first as it is used by
Ceph in a couple of ways:
crc32_iscsi_01 explicitly handles and checks for < 8
byte buffers and computes the CRC32C value using the
hardware-accelerated CRC32 instruction. In comparison,
crc32_iscsi_00 prefetches too far in cases of small
buffers, requiring the Ceph code to explicitly check
and handle this case differently in software. This
software-fallback implementation of CRC32 also comes
with a different set of LUTs (look up tables) and is
less efficient as it does not make use of the CRC32
instruction.
crc32_iscsi_00 makes use of large LUTs (look up
tables) to effectively perform the modular reduction
required to produce the CRC32C value. In constrast,
crc32_iscsi_01 uses the PCLMUL instruction set to
perform reductions 128-bits at a time with smaller
LUTs, resulting in greater throughput and less data
cache pollution.
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
x
between the brackets:[x]
. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e