Re: [PATCH v2 2/4] blk-crypto: make blk_crypto_evict_key() more robust

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?