[PATCH] dm-crypt: Document new no_workqueue flags.

[PATCH] dm-crypt: Document new no_workqueue flags.

[Milan Broz]

Patch 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877 introduced new
dm-crypt no_read_workqueue and no_write_workqueue flags.

This patch adds documentation to admin guide for them.

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 Documentation/admin-guide/device-mapper/dm-crypt.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
index 8f4a3f889d43..94466921415d 100644
--- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -121,6 +121,12 @@ submit_from_crypt_cpus
     thread because it benefits CFQ to have writes submitted using the
     same context.
 
+no_read_workqueue
+    Bypass dm-crypt internal workqueue and process read requests synchronously.
+
+no_write_workqueue
+    Bypass dm-crypt internal workqueue and process write requests synchronously.
+
 integrity:<bytes>:<type>
     The device requires additional <bytes> metadata per-sector stored
     in per-bio integrity structure. This metadata must by provided
-- 
2.28.0

Re: [PATCH] dm-crypt: Document new no_workqueue flags.

[Damien Le Moal]

On 2020/08/21 2:46, Milan Broz wrote:
> Patch 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877 introduced new
> dm-crypt no_read_workqueue and no_write_workqueue flags.
> 
> This patch adds documentation to admin guide for them.
> 
> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> ---
>  Documentation/admin-guide/device-mapper/dm-crypt.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> index 8f4a3f889d43..94466921415d 100644
> --- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
> +++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> @@ -121,6 +121,12 @@ submit_from_crypt_cpus
>      thread because it benefits CFQ to have writes submitted using the
>      same context.
>  
> +no_read_workqueue
> +    Bypass dm-crypt internal workqueue and process read requests synchronously.
> +
> +no_write_workqueue
> +    Bypass dm-crypt internal workqueue and process write requests synchronously.

Could you add one more line here saying something like:

This option is automatically enabled for host-managed zoned block devices (e.g.
host-managed SMR hard-disks).

Thanks !

> +
>  integrity:<bytes>:<type>
>      The device requires additional <bytes> metadata per-sector stored
>      in per-bio integrity structure. This metadata must by provided
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] dm-crypt: Document new no_workqueue flags.

[Milan Broz]

On 24/08/2020 03:15, Damien Le Moal wrote:
> On 2020/08/21 2:46, Milan Broz wrote:
>> Patch 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877 introduced new
>> dm-crypt no_read_workqueue and no_write_workqueue flags.
>>
>> This patch adds documentation to admin guide for them.
>>
>> Signed-off-by: Milan Broz <gmazyland@gmail.com>
>> ---
>>  Documentation/admin-guide/device-mapper/dm-crypt.rst | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
>> index 8f4a3f889d43..94466921415d 100644
>> --- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
>> +++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
>> @@ -121,6 +121,12 @@ submit_from_crypt_cpus
>>      thread because it benefits CFQ to have writes submitted using the
>>      same context.
>>  
>> +no_read_workqueue
>> +    Bypass dm-crypt internal workqueue and process read requests synchronously.
>> +
>> +no_write_workqueue
>> +    Bypass dm-crypt internal workqueue and process write requests synchronously.
> 
> Could you add one more line here saying something like:
> 
> This option is automatically enabled for host-managed zoned block devices (e.g.
> host-managed SMR hard-disks).

Yes, Mike can squeeze it there (Mike, if you prefer, I can resend the patch with the note above).

I just see one problem here - when we activate device without these flags,
then dm-crypt decides to set the feature bits because of zone device.

So you activate device with some feature set but table will show something different.
It is not a technical problem, but I think DM table was not expected to behave this way.

It is probably not the first change (I think some flags are activated in dm-integrity
according to on-disk superblock state only).

Milan

p.s. Both noqueue flags are now supported in cryptsetup master, for LUKS2 we can store them persistently
(IOW to be used in all later LUKS2 activations if kernel supports it).
It should appear in next stable cryptsetup release.

Re: [PATCH] dm-crypt: Document new no_workqueue flags.

[Damien Le Moal]

On 2020/08/24 15:46, Milan Broz wrote:
> On 24/08/2020 03:15, Damien Le Moal wrote:
>> On 2020/08/21 2:46, Milan Broz wrote:
>>> Patch 39d42fa96ba1b7d2544db3f8ed5da8fb0d5cb877 introduced new dm-crypt
>>> no_read_workqueue and no_write_workqueue flags.
>>> 
>>> This patch adds documentation to admin guide for them.
>>> 
>>> Signed-off-by: Milan Broz <gmazyland@gmail.com> --- 
>>> Documentation/admin-guide/device-mapper/dm-crypt.rst | 6 ++++++ 1 file
>>> changed, 6 insertions(+)
>>> 
>>> diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst
>>> b/Documentation/admin-guide/device-mapper/dm-crypt.rst index
>>> 8f4a3f889d43..94466921415d 100644 ---
>>> a/Documentation/admin-guide/device-mapper/dm-crypt.rst +++
>>> b/Documentation/admin-guide/device-mapper/dm-crypt.rst @@ -121,6 +121,12
>>> @@ submit_from_crypt_cpus thread because it benefits CFQ to have writes
>>> submitted using the same context.
>>> 
>>> +no_read_workqueue +    Bypass dm-crypt internal workqueue and process
>>> read requests synchronously. + +no_write_workqueue +    Bypass dm-crypt
>>> internal workqueue and process write requests synchronously.
>> 
>> Could you add one more line here saying something like:
>> 
>> This option is automatically enabled for host-managed zoned block devices
>> (e.g. host-managed SMR hard-disks).
> 
> Yes, Mike can squeeze it there (Mike, if you prefer, I can resend the patch
> with the note above).
> 
> I just see one problem here - when we activate device without these flags, 
> then dm-crypt decides to set the feature bits because of zone device.
> 
> So you activate device with some feature set but table will show something
> different. It is not a technical problem, but I think DM table was not
> expected to behave this way.

Hmmm. Never wondered about that... Mike ? Is this a big problem ?

> It is probably not the first change (I think some flags are activated in
> dm-integrity according to on-disk superblock state only).
> 
> Milan
> 
> p.s. Both noqueue flags are now supported in cryptsetup master, for LUKS2 we
> can store them persistently (IOW to be used in all later LUKS2 activations if
> kernel supports it). It should appear in next stable cryptsetup release.

Great. Thanks. Note that I have code for Luks format on host-managed SMR disks,
or rather any host-managed zoned block device, including ZNS NVMe SSDs.

The current cryptsetup code needs some massaging for these drives because:
1) offset+size needs to be zoned aligned, so at the very least defaulting to
--offset=<device zone size> is needed.
2) The on-disk formatting is annoyingly not writing sequentially :) So the
current code fails if the drive does not have a conventional zone at LBA 0.

My code is rather dirty for now and needs a good cleanup. I will do that
rebasing on the next stable that adds the no queue flags support. The current
cryptsetup does work as is with plain type, and with Luks type for SMR drives
that have a conventional zone at LBA 0 with the added --offset=<device zone
size> option specified.

Best regards.


-- 
Damien Le Moal
Western Digital Research