[PATCH v3 01/28] crypto: change backlog return code to -EIOCBQUEUED

From
Gilad Ben-Yossef <gilad@benyossef.com>
Date
2017-07-03 13:23:28
Also in
dm-devel, keyrings, linux-cifs, linux-crypto, linux-fscrypt, linux-mediatek, linux-raid, linux-security-module, lkml
Reply to
Herbert Xu, 2017-07-03
On Mon, Jul 3, 2017 at 3:35 PM, Herbert Xu [off-list ref] wrote:
On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
quoted
The crypto API was using the -EBUSY return value to indicate
both a hard failure to submit a crypto operation into a
transformation provider when the latter was busy and the backlog
mechanism was not enabled as well as a notification that the operation
was queued into the backlog when the backlog mechanism was enabled.

Having the same return code indicate two very different conditions
depending on a flag is both error prone and requires extra runtime
check like the following to discern between the cases:

      if (err == -EINPROGRESS ||
          (err == -EBUSY && (ahash_request_flags(req) &
                             CRYPTO_TFM_REQ_MAY_BACKLOG)))

This patch changes the return code used to indicate a crypto op
was queued in the backlog to -EIOCBQUEUED, thus resolving both
issues.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
So you're changing the success case from EBUSY to EIOCBQUEUED.
This results in a lot of churn as you have to change every single
driver and caller.

How about changing the error case to use something other than
EBUSY instead?
That was indeed my first thought - I wanted to change EBUSY to EAGAIN.
It might even be a more descriptive error message since the failure is
transient.

What stopped me was that I saw the algif interface passes this EBUSY value
to user space. I don't know of any software that depends on this value but
I'm really averse to changing user space API.

Of course, we can just plant a (ret != AGAIN : ? EBUSY) in the algif interface
return to user space code path but that felt like a kludge to me.

And it doesn't really save any churn, I think - I'd still have to
visit all the places that
use the flags value to distinguish between the two EBUSY use cases and  I need
to visit most places that check for EBUSY as backlog indication because I'm
replacing the check with the generic replacement, so it doesn't look
like in the end
it will be a smaller change.

Having said that, if you prefer me to replace the error case I'd
happily do that.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
Thread (31 messages) 31 messages, 2 authors, last reply 2017-07-03
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help