Re: [PATCH 0/2] Introduce the request handling for dm-crypt

From: Baolin Wang
Date: Wed Nov 11 2015 - 21:36:56 EST


On 12 November 2015 at 02:18, Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> On Wed, Nov 11 2015 at 4:31am -0500,
> Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>
>> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> decrypt block data, which can only hanle one bio at one time. As we know,
>> one bio must use the sequential physical address and it also has a limitation
>> of length. Thus it may limit the big block encyrtion/decryption when some
>> hardware support the big block data encryption.
>>
>> This patch series introduc the 'based-request' method to handle the data
>> encryption/decryption. One request can contain multiple bios, so it can
>> handle big block data to improve the efficiency.
>
> The duality of bio-based vs request-based code paths in DM core frankly
> sucks. So the prospect of polluting dm-crypt with a similar duality is
> really _not_ interesting.
>

That's right. But we'll not introduce the duality things, cause we
will remove the bio based things in dm-crypt if the request based
things are accepted.

> Request-based DM requires more memory reserves per device than bio-based
> DM. Also, you cannot stack request-based DM ontop of bio-based devices
> (be them DM, MD, etc) so request-based DM's underlying storage stack
> gets a lot less interesting with this change.
>

Yes, the request based requires more memory than bio based, but it is
not too much. And the request based has a big performance improvement.

> That said, it could be that the benefits of supporting both bio-based
> and request-based DM in dm-crypt outweigh any overhead/limitations. But
> you haven't given any performance data to justify this patchset.
>

Like I said above, we plan to remove the bio based things which are
not good support for hardware engine encryption. And I'll show you the
performance data to prove the request things have a good performance.

> There needs to be a _really_ compelling benefit to do this.
>
> Also, FYI, having a big CONFIG knob to switch all of dm-crypt from
> bio-based to request-based is _not_ acceptable. Both modes would need
> to be supported in parallel. Could easily be that not all devices in a
> system will benefit from being request-based.
>

OK. The CONFIG is not suitable here. I'll remove the CONFIG with just
enable the request based things.

> Regardless, the risk of this change causing request-based DM to become
> more brittle than it already is concerns me.
>
> But I'm trying to keep an open mind... show me data that real hardware
> _really_ benefits and we'll go from there. Again, it needs to be "OMG,
> this is amazing!" level performance to warrant any further serious
> consideration.

OK. I'll show you the performance data. Thanks for your comments.



--
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/