Re: [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices

From: Mike Snitzer <snitzer@redhat.com>
To: Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>
Cc: linux-block@vger.kernel.org,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	"ssudhakarp@gmail.com" <ssudhakarp@gmail.com>,
	Martin Petersen <martin.petersen@oracle.com>,
	mst@redhat.com, "dm-crypt@saout.de" <dm-crypt@saout.de>,
	Eric Biggers <ebiggers@kernel.org>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	"agk@redhat.com" <agk@redhat.com>,
	linux-scsi@vger.kernel.org, Shirley Ma <shirley.ma@oracle.com>,
	Milan Broz <gmazyland@gmail.com>,
	Mike Christie <michael.christie@oracle.com>
Subject: Re: [RFC PATCH 0/2] dm crypt: Allow unaligned buffer lengths for skcipher devices
Date: Fri, 25 Sep 2020 16:15:12 -0400	[thread overview]
Message-ID: <20200925201512.GA6025@redhat.com> (raw)
In-Reply-To: <MWHPR04MB37588DF8C3FFF4BD0C3CD543E7360@MWHPR04MB3758.namprd04.prod.outlook.com>

On Thu, Sep 24 2020 at  9:09pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> On 2020/09/25 4:14, Sudhakar Panneerselvam wrote:
> >>
> >> On Thu, 24 Sep 2020, Sudhakar Panneerselvam wrote:
> >>
> >>>> By copying it to a temporary aligned buffer and issuing I/O on this
> >>>> buffer.
> >>>
> >>> I don't like this idea. Because, you need to allocate additional pages
> >>> for the entire I/O size(for the misaligned case, if you think through
> >>
> >> You can break the I/O to smaller pieces. You can use mempool for
> >> pre-allocation of the pages.
> > 
> > Assuming we do this, how is this code simpler(based on your
> > comment below) than the fix in dm-crypt? In fact, this approach 
> > would make the code change look bad in vhost, at the same time
> > having performance penalty. By doing this, we are just moving the 
> > responsibility to other unrelated component.
> 
> Because vhost is at the top of the block-io food chain. Fixing the unaligned
> segments there will ensure that it does not matter what device is under it. It
> will work.

Right, I agree. This should be addressed in vhost-scsi.  And vhost-scsi
probably needs to be interfacing through block core to submit IO that
respects the limits of its underlying block device.

So please lift your proposed dm-crypt changes to vhost-scsi:
https://patchwork.kernel.org/patch/11781207/
https://patchwork.kernel.org/patch/11781053/

Maybe work with vhost-scsi maintainers to see about making the code
reusable in block core; so that any future unaligned application IO is
dealt in other drivers using the same common code.

But I'm not interested in taking these changes into dm-crypt:

NAK

> I am still baffled that the unaligned segments go through in the first place...
> Do we have something missing in the BIO code ?

Cc'ing linux-block, could be.

Thanks,
Mike