On 5/5/2015 4:15 PM, Rabin Vincent wrote: > This reverts commit 0618764cb25f6fa9fb31152995de42a8a0496475. > > The problem which that commit attempts to fix actually lies in the > Freescale CAAM crypto driver. > > dm-crypt uses CRYPTO_TFM_REQ_MAY_BACKLOG. This means the the crypto > driver should internally backlog requests which arrive when the queue is > full and process them later. Until the crypto hw's queue becomes full, > the driver returns -EINPROGRESS. When the crypto hw's queue if full, > the driver returns -EBUSY, and if CRYPTO_TFM_REQ_MAY_BACKLOG is set, is > expected to backlog the request and process it when the hardware has > queue space. At the point when the driver takes the request from the > backlog and starts processing it, it calls the completion function with > a status of -EINPROGRESS. The completion function is called (for a > second time, in the case of backlogged requests) with a status/err of 0 > when a request is done. > > Crypto drivers for hardware without hardware queueing use the helpers, > crypto_init_queue(), crypto_enqueue_request(), crypto_dequeue_request() > and crypto_get_backlog() helpers to implement this behaviour correctly, > while others implement this behaviour without these helpers (ccp, for > example). > > dm-crypt (before this patch) uses this API correctly. It queues up as > many requests as the hw queues will allow (i.e. as long as it gets back > -EINPROGRESS from the request function). Then, when it sees at least > one backlogged request (gets -EBUSY), it waits till that backlogged > request is handled (completion gets called with -EINPROGRESS), and then > continues. The references to af_alg_wait_for_completion() and > af_alg_complete() in that commit's commit message are irrelevant because > those functions only handle one request at a time, unlink dm-crypt. > > The problem is that the Freescale CAAM driver, which that commit > describes as having being tested with, fails to implement the > backlogging behaviour correctly. In cam_jr_enqueue(), if the hardware > queue is full, it simply returns -EBUSY without backlogging the request. > What the observed deadlock was is not described in the commit message > but it is obviously the wait_for_completion() in crypto_convert() where > dm-crypto would wait for the completion being called with -EINPROGRESS > in the case of backlogged requests. This completion will never be > completed due to the bug in the CAAM driver. > > What that commit does is that it makes dm-crypt wait for every request, > even when the driver/hardware queues are not full, which means that > dm-crypt will never see -EBUSY. This means that that commit will cause > a performance regression on all crypto drivers which implement the API > correctly. > > Revert it. Correct backlog handling should be implemented in the CAAM > driver instead. > > Signed-off-by: Rabin Vincent <rabin.vincent@xxxxxxxx> Reviewed-by: Horia Geanta <horia.geanta@xxxxxxxxxxxxx> I confirm CAAM crypto driver currently lacks backlogging support. Horia -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
