On Tue, May 05 2015 at 2:42am -0400, Milan Broz <mbroz@xxxxxxxxxx> wrote: > On 05/05/2015 05:22 AM, Mike Snitzer wrote: > > On Mon, May 04 2015 at 5:32pm -0400, > > Rabin Vincent <rabin@xxxxxx> wrote: > > > >> On Sat, May 02, 2015 at 09:03:28PM +0200, Greg Kroah-Hartman wrote: > >>> 3.14-stable review patch. If anyone has any objections, please let me know. > >>> > >>> ------------------ > >>> > >>> From: Ben Collins <ben.c@xxxxxxxxxxxx> > >>> > >>> commit 0618764cb25f6fa9fb31152995de42a8a0496475 upstream. > >> > >> The commit in question was recently merged to mainline and is now being > >> sent to various stable kernels. But I think the patch is wrong and the > >> real problem lies in the Freescale CAAM driver which is described as > >> having being tested with. > ... > > >> 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. > > > Mike, I thought there was a discussion with crypto API maintainer before > merging this patch, I think there were some comments that the patch seems > to be not correct... I unfortunately didn't cc Herbert on the original v2 patch (but I did later bounce the mail to him IIRC). Regardless, no I didn't see any feedback and I really should've been more active in engaging him. > This API (EBUSY/EINPROGESS codes) use was there since dmcrypt switched > to async crypto API. > > There should tests for it (some years ago we tested the async path by forcing > to use cryptd, this was able to clearly simulate the BUSY path as well), > but not sure if it is still possible so easily. > > > Any chance you'd be willing to provide in-code comments in the > > appropriate places in dm-crypt.c (after having reverted this patch in > > question)? > > > > I'd be happy to take the combination of the revert and documentation in > > a single patch and get it to Linus for 4.0-rc3. > > Please, do not mix revert patches with additions (even if it is just comment), > someone could be even more confused what's going on here. I was only saying to mix comments and revert if the patch in question didn't get into stable@ at all. But safer to just do a pure revert. I get it queued up to send to Linus. > If nobody volunteers, I'll try to add some comments later to dmcrypt code, > but that is definitely not for stable. Yeap, thanks. -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html