Enable SIMD for encryption #9296
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implemention even when execute by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For examples, when performing a read from the ARC. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Codecov Report
@@ Coverage Diff @@
## master #9296 +/- ##
==========================================
+ Coverage 79.12% 79.3% +0.17%
==========================================
Files 401 401
Lines 122063 122071 +8
==========================================
+ Hits 96587 96812 +225
+ Misses 25476 25259 -217
Continue to review full report at Codecov.
|
|
Thanks for doing this. I'll need a couple of days to build a version with your commit cherry picked. Once I'm done I'll report back. |
behlendorf
added
Status: Code Review Needed
|
@AttilaFueloep that would be great, thanks! |
tcaputi
approved these changes
Sep 9, 2019
I'm working on running those tests now, I should have some numbers to share soon.
That's right. For any kernel where the fpu symbols are available we don't do the dispatch in |
|
@behlendorf If you tell me how to generate results I'd be more than happy to try this on my machine |
The results are as expected. The performance gain from using the optimized versions (40-50%) significantly outweighs the negligible cost of dispatching the operation to the IO pipeline (<-0.5%).
@lovesegfault additional testing would be welcome. Benchmarking your performance both before and after applying this commit should be enable to determine if it's working as intended. |
|
I am also running encryption on my root filesystem (aes-128-gcm) and am experiencing pretty poor performance. I would be happy to perform some testing if required. I am running this branch now but performance is still low. Let me know if I can assist at all. |
|
@dmimpact one thing worth trying if you're running an amd system is to set the |
behlendorf
added
Status: Accepted
I'm also seeing slightly (< 10%) better performance with the |
|
@AttilaFueloep thanks for the additional data point. It sounds as if adding a micro benchmark to dynamically select the fastest available algorithms for the icp would be worthwhile. |
|
I just rebooted my system with this patch applied. While there is a very noticeable difference, |
|
@behlendorf Could |
Thanks for the additional testing of the patch, I'm glad to hear it helps as intended. This change won't effect the output |
|
@behlendorf Oh! I'm surprised that |
mattmacy
pushed a commit
to zfsonfreebsd/ZoF
that referenced
this pull request
Sep 10, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
|
@behlendorf Thanks, Unfortunately I am not running AMD but rather Intel Coffee Lake. Performance does feel a bit better. but is not what I would expect from my cpu + storage config (i7 + PCIe NVME). Regardless I am glad to see this issue has received attention and will test this out. |
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 17, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 18, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 18, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 18, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Sep 19, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
behlendorf
added a commit
to behlendorf/zfs
that referenced
this pull request
Oct 4, 2019
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
stevijo
pushed a commit
to stevijo/zfs
that referenced
this pull request
Jan 12, 2020
When adding the SIMD compatibility code in e5db313 the decryption of a dataset wrapping key was left in a user thread context. This was done intentionally since it's a relatively infrequent operation. However, this also meant that the encryption context templates were initialized using the generic operations. Therefore, subsequent encryption and decryption operations would use the generic implementation even when executed by an I/O pipeline thread. Resolve the issue by initializing the context templates in an I/O pipeline thread. And by updating zio_do_crypt_uio() to dispatch any encryption operations to a pipeline thread when called from the user context. For example, when performing a read from the ARC. Tested-by: Attila Fülöp <attila@fueloep.org> Reviewed-by: Tom Caputi <tcaputi@datto.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#9215 Closes openzfs#9296
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Issue #9215.
Description
When adding the SIMD compatibility code in e5db313 the decryption of a
dataset wrapping key was left in a user thread context. This was done
intentionally since it's a relatively infrequent operation. However,
this also meant that the encryption context templates were initialized
using the generic operations. Therefore, subsequent encryption and
decryption operations would use the generic implementation even when
execute by an I/O pipeline thread.
Resolve the issue by initializing the context templates in an I/O
pipeline thread. And by updating zio_do_crypt_uio() to dispatch any
encryption operations to a pipeline thread when called from the user
context. For example, when performing a read from the ARC.
How Has This Been Tested?
Manually by instrumenting the code to ensure the encryption SIMD operations
were never performed in an incorrect thread context. Verified the expected
implementation was being used (aesni).
Submitted to the CI for a full run of the test suite.
Types of changes
Checklist:
Signed-off-by.