-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Linux 4.13+: Warnings from objtool while compiling the 'icp' kernel module: "unsupported stack pointer realignment" (in SHA1/SHA2 functions) #6950
Comments
Also, I thought it'd be worth mentioning that the asm implementations of SHA1/SHA2 in the ICP module come from Illumos, which in turn got them from OpenSSL; quite a long time ago, by the looks of it (2009 at the very latest). It looks like the upstream OpenSSL implementation has advanced quite a bit since Illumos branched their code off; among other things, the OpenSSL version now has specializations for SSSE3, AVX, AVX2+BMI, and Intel SHA instruction set extensions now. So that might be worth looking at sometime. (And/or #2351.) |
Still outstanding as of latest git. Funny thing is this was the first hit on google for that error :) |
…ecompression With compressed ARC (bug openzfs#6950) we use up to 25% of our CPU to decompress indirect blocks, under a workload of random cached reads. To reduce this decompression cost, we would like to increase the size of the dbuf cache so that more indirect blocks can be stored uncompressed. If we are caching entire large files of recordsize=8K, the indirect blocks use 1/64th as much memory as the data blocks (assuming they have the same compression ratio). We suggest making the dbuf cache be 1/32nd of all memory, so that in this scenario we should be able to keep all the indirect blocks decompressed in the dbuf cache. (We want it to be more than the 1/64th that the indirect blocks would use because we need to cache other stuff in the dbuf cache as well.) In real world workloads, this won't help as dramatically as the example above, but we think it's still worth it because the risk of decreasing performance is low. The potential negative performance impact is that we will be slightly reducing the size of the ARC (by ~3%). Porting Notes: * Added modules options to zfs-module-parameters.5 man page. * Preserved scaling based on target ARC size rather than max ARC size. Authored by: George Wilson <[email protected]> Reviewed by: Dan Kimmel <[email protected]> Reviewed by: Prashanth Sreenivasa <[email protected]> Reviewed by: Paul Dagnelie <[email protected]> Ported-by: Brian Behlendorf <[email protected]> OpenZFS-issue: https://www.illumos.org/issues/9188 OpenZFS-commit: openzfs/openzfs#564 Upstream bug: DLPX-46942
Confirmed on Debian testing with ZFS 0.7.11, custom Kernel 4.19.1
|
What are the consequences of this? Is it safe to ignore? |
can confirm this for
|
Yes, it's safe to ignore. Though of course we would like to resolve the warning. |
This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions. |
This is still a thing on Arch Linux GCC 10.2 against 5.10.12:
|
Fix it, the kernel will be adopting this fix soon: "x86/crypto/asm: objtool support" |
Thanks to the excellent information in this ticket, I'd say |
I'm fine with silencing the warning for all the reasons you mentioned. |
OK, will open a PR once #11733 settles. |
"objtool, modules: Discard objtool annotation sections for modules" I use such a patch on my local test branch, there are no problems, everything works and the build goes without any warnings. diff --git a/fs/zfs/icp/asm-x86_64/modes/aesni-gcm-x86_64.S b/fs/zfs/icp/asm-x86_64/modes/aesni-gcm-x86_64.S
index dc71ae2..b7f0296 100644
--- a/fs/zfs/icp/asm-x86_64/modes/aesni-gcm-x86_64.S
+++ b/fs/zfs/icp/asm-x86_64/modes/aesni-gcm-x86_64.S
@@ -1239,6 +1239,7 @@
ret
.size atomic_toggle_boolean_nv,.-atomic_toggle_boolean_nv
+.data
.align 64
.Lbswap_mask:
.byte 15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0
@@ -1253,6 +1254,13 @@
.byte 65,69,83,45,78,73,32,71,67,77,32,109,111,100,117,108,101,32,102,111,114,32,120,56,54,95,54,52,44,32,67,82,89,80,84,79,71,65,77,83,32,98,121,32,60,97,112,112,114,111,64,111,112,101,110,115,115,108,46,111,114,103,62,0
.align 64
+
+.pushsection .discard.func_stack_frame_non_standard, "aw"
+.long _aesni_ctr32_ghash_no_movbe_6x - .
+.long aesni_gcm_decrypt - .
+.long aesni_gcm_encrypt - .
+.popsection
+
/* Mark the stack non-executable. */
#if defined(__linux__) && defined(__ELF__)
.section .note.GNU-stack,"",%progbits
diff --git a/fs/zfs/icp/asm-x86_64/modes/ghash-x86_64.S b/fs/zfs/icp/asm-x86_64/modes/ghash-x86_64.S
index 90cc36b..501a3a5 100644
--- a/fs/zfs/icp/asm-x86_64/modes/ghash-x86_64.S
+++ b/fs/zfs/icp/asm-x86_64/modes/ghash-x86_64.S
@@ -652,6 +652,8 @@
.byte 0xf3,0xc3
.cfi_endproc
.size gcm_ghash_avx,.-gcm_ghash_avx
+
+.data
.align 64
.Lbswap_mask:
.byte 15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0
diff --git a/fs/zfs/icp/asm-x86_64/sha1/sha1-x86_64.S b/fs/zfs/icp/asm-x86_64/sha1/sha1-x86_64.S
index cb92378..da854a3 100644
--- a/fs/zfs/icp/asm-x86_64/sha1/sha1-x86_64.S
+++ b/fs/zfs/icp/asm-x86_64/sha1/sha1-x86_64.S
@@ -1348,6 +1348,10 @@
#endif /* lint || __lint */
+.pushsection .discard.func_stack_frame_non_standard, "aw"
+.long sha1_block_data_order - .
+.popsection
+
#ifdef __ELF__
.section .note.GNU-stack,"",%progbits
#endif
diff --git a/fs/zfs/icp/asm-x86_64/sha2/sha256_impl.S b/fs/zfs/icp/asm-x86_64/sha2/sha256_impl.S
index 766b753..57867fc 100644
--- a/fs/zfs/icp/asm-x86_64/sha2/sha256_impl.S
+++ b/fs/zfs/icp/asm-x86_64/sha2/sha256_impl.S
@@ -2058,6 +2058,10 @@
.long 0x90befffa,0xa4506ceb,0xbef9a3f7,0xc67178f2
#endif /* !lint && !__lint */
+.pushsection .discard.func_stack_frame_non_standard, "aw"
+.long SHA256TransformBlocks - .
+.popsection
+
#ifdef __ELF__
.section .note.GNU-stack,"",%progbits
#endif
diff --git a/fs/zfs/icp/asm-x86_64/sha2/sha512_impl.S b/fs/zfs/icp/asm-x86_64/sha2/sha512_impl.S
index 6e37618..1239d53 100644
--- a/fs/zfs/icp/asm-x86_64/sha2/sha512_impl.S
+++ b/fs/zfs/icp/asm-x86_64/sha2/sha512_impl.S
@@ -2083,6 +2083,10 @@
.quad 0x5fcb6fab3ad6faec,0x6c44198c4a475817
#endif /* !lint && !__lint */
+.pushsection .discard.func_stack_frame_non_standard, "aw"
+.long SHA512TransformBlocks - .
+.popsection
+
#ifdef __ELF__
.section .note.GNU-stack,"",%progbits
#endif
|
Actually I'd prefer to use the C macro |
For build with Clang + LTO, this doesn't work: I've provided my variant for Linux. I have no other systems. It also allowed us to move bswap_mask to the .data section and get rid of the "can't find jump dest instruction" warning. |
Much appreciated. Regarding bswap_mask please see #11733 (comment). Not sure if we officially support clang on Linux? But I'll have a look anyhow. |
Clang + LTO build support has already been added to the Linux tree. |
Objtool requires the use of a DRAP register while aligning the stack. Since a DRAP register is a gcc concept and we are notoriously low on registers in the crypto code, it's not worth the effort to mimic gcc generated stack realignment. We simply silence the warning by adding the offending object files to OBJECT_FILES_NON_STANDARD. Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#6950
Objtool requires the use of a DRAP register while aligning the stack. Since a DRAP register is a gcc concept and we are notoriously low on registers in the crypto code, it's not worth the effort to mimic gcc generated stack realignment. We simply silence the warning by adding the offending object files to OBJECT_FILES_NON_STANDARD. Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#6950
Objtool requires the use of a DRAP register while aligning the stack. Since a DRAP register is a gcc concept and we are notoriously low on registers in the crypto code, it's not worth the effort to mimic gcc generated stack realignment. We simply silence the warning by adding the offending object files to OBJECT_FILES_NON_STANDARD. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#6950 Closes openzfs#11914
Objtool requires the use of a DRAP register while aligning the stack. Since a DRAP register is a gcc concept and we are notoriously low on registers in the crypto code, it's not worth the effort to mimic gcc generated stack realignment. We simply silence the warning by adding the offending object files to OBJECT_FILES_NON_STANDARD. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Attila Fülöp <[email protected]> Closes openzfs#6950 Closes openzfs#11914
I don't know how important these build-time warnings actually are, but I've typed up a full report here anyway.
The warnings
I was building the latest zfs master, 4e9b156, for Linux 4.14.5, on x86_64 today.
Here's what came up during the build process:
Those warnings correspond to these particular instructions from the objdump'd object files:
The recent update to objtool
It looks like this warning will only appear when compiling for Linux 4.13 or later, since the objtool patchset that added new warnings (including the "unsupported stack pointer realignment" one) was merged into the mainline kernel in 4.13.
Here's some additional info on that objtool patchset:
Here's the specific part of the objtool code that's complaining.
And here's some internal documentation regarding what it's complaining about.
The ZoL source lines being complained about
Here are the relevant assembly code source file lines in ZoL:
https://github.com/zfsonlinux/zfs/blob/4e9b156960562373e005798575a3fbc6d66e32ff/module/icp/asm-x86_64/sha1/sha1-x86_64.S#L79
https://github.com/zfsonlinux/zfs/blob/4e9b156960562373e005798575a3fbc6d66e32ff/module/icp/asm-x86_64/sha2/sha256_impl.S#L96
https://github.com/zfsonlinux/zfs/blob/4e9b156960562373e005798575a3fbc6d66e32ff/module/icp/asm-x86_64/sha2/sha512_impl.S#L97
32-bit
As far as I can tell, i386 builds of zfs won't emit this warning, as from what I can see, the asm implementations of the three SHA functions in question are only used for the kernel module on x86_64, and everything else just uses the C implementations.
But I don't have an i386 Linux box/VM ready-at-hand to do a quick build and actually confirm for sure that these warnings don't happen there.
The text was updated successfully, but these errors were encountered: