On Wed, Mar 08, 2023 at 11:36:43AM -0800, Eric Biggers wrote:
> const struct blk_crypto_key *key)
> @@ -389,22 +377,22 @@ int __blk_crypto_evict_key(struct blk_crypto_profile *profile,
>
> blk_crypto_hw_enter(profile);
> slot = blk_crypto_find_keyslot(profile, key);
> + if (slot) {
Isn't !slot also an error condition that we should warn on?
> + if (WARN_ON_ONCE(atomic_read(&slot->slot_refs) != 0)) {
> + /* BUG: key is still in use by I/O */
> + err = -EBUSY;
Either way two gotos to jump forward for the error case would improve
the readability of the code a fair bit I think.
> + } else {
> + err = profile->ll_ops.keyslot_evict(
> + profile, key,
> + blk_crypto_keyslot_index(slot));
> + }
> + /*
> + * Callers may free the key even on error, so unlink the key
> + * from the hash table and clear slot->key even on error.
> + */
> + hlist_del(&slot->hash_node);
> + slot->key = NULL;
> }
> blk_crypto_hw_exit(profile);
> return err;
Also given your above accurate description of the calling context,
what is the point of even returning an error here vs just logging
an error message?
