Retry OCF ENOMEM errors in ZFS
AcceptedPublic
Actions

Authored by mav on May 18 2021, 7:39 PM.

Details

Reviewers

jhb
asomers
markj

Summary

ZFS does not expect transient errors from crypto. For read they are counted as checksum errors, while for write end up in panic. To not panic on random low memory conditions retry ENOMEM errors in OCF wrapper function.

While there remove unneeded timeout and priority parameters from msleep().

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mav created this revision.May 18 2021, 7:39 PM

Herald added a subscriber: delphij. · View Herald TranscriptMay 18 2021, 7:39 PM

mav requested review of this revision.May 18 2021, 7:39 PM

markj accepted this revision.May 18 2021, 7:53 PM

markj added inline comments.

sys/contrib/openzfs/module/os/freebsd/zfs/crypto_os.c
177	BTW, the locking here and freebsd_zfs_crypt_done() is unnecessary for synchronous drivers like aesni. Same for the `wakeup()` call. You can check `CRYPTO_SESS_SYNC(session->fs_sid)` to determine if this is the case, if profiles suggest it is worth doing.
181	Presumably it should be `else if`.
193	This does not apply to the ENOMEM case, but I think it's harmless.

This revision is now accepted and ready to land.May 18 2021, 7:53 PM

asomers added inline comments.May 18 2021, 7:56 PM

sys/contrib/openzfs/module/os/freebsd/zfs/crypto_os.c
180	You should probably `continue` after the `pause`, don't you think?

mav added inline comments.May 18 2021, 8:15 PM

sys/contrib/openzfs/module/os/freebsd/zfs/crypto_os.c
177	I haven't profiled this code very much, but I have suspect it won't be visible, comparing to cryptography.
180	No, I meant "else" there.
181	Yes. Thanks.