On Tue, 2017-07-18 at 17:52 -0700, Tim Chen wrote: > On 07/17/2017 10:41 PM, Herbert Xu wrote: > > On Tue, Jun 27, 2017 at 05:26:13PM -0700, Megha Dey wrote: > >> > >> +static void completion_callback(struct mcryptd_skcipher_request_ctx *rctx, > >> + struct mcryptd_alg_cstate *cstate, > >> + int err) > >> +{ > >> + struct skcipher_request *req = cast_mcryptd_ctx_to_req(rctx); > >> + > >> + /* remove from work list and invoke completion callback */ > >> + spin_lock(&cstate->work_lock); > >> + list_del(&rctx->waiter); > >> + spin_unlock(&cstate->work_lock); > >> + > >> + if (irqs_disabled()) > >> + rctx->complete(&req->base, err); > >> + else { > >> + local_bh_disable(); > >> + rctx->complete(&req->base, err); > >> + local_bh_enable(); > >> + } > >> +} > > > > The fact that you need to do this check means that this design is > > wrong. You should always know what context you are in. > > > > I think you are right. The irqs_disabled check is not necessary > as we only call this function in the context of the mcryptd thread. > When I wrote the original mb algorithms I was probably unsure > and put this check in as a precaution in other mb algorithms and > Megha did the same. I will make this change. > > >> +/* > >> + * CRYPTO_ALG_ASYNC flag is passed to indicate we have an ablk > >> + * scatter-gather walk. > >> + */ > >> +static struct skcipher_alg aes_cbc_mb_alg = { > >> + .base = { > >> + .cra_name = "cbc(aes)", > >> + .cra_driver_name = "cbc-aes-aesni-mb", > >> + .cra_priority = 500, > >> + .cra_flags = CRYPTO_ALG_INTERNAL, > >> + .cra_blocksize = AES_BLOCK_SIZE, > >> + .cra_ctxsize = CRYPTO_AES_CTX_SIZE, > >> + .cra_module = THIS_MODULE, > >> + }, > >> + .min_keysize = AES_MIN_KEY_SIZE, > >> + .max_keysize = AES_MAX_KEY_SIZE, > >> + .ivsize = AES_BLOCK_SIZE, > >> + .setkey = aes_set_key, > >> + .encrypt = mb_aes_cbc_encrypt, > >> + .decrypt = mb_aes_cbc_decrypt > >> +}; > > > > So this claims to be a sync algorithm. Is this really the case? yes, the inner algorithm is sync whereas the outer algorithm is async. > > > > Cheers, > > >