Re: [dm-devel] [PATCH v5 00/11] dm: Improve zoned block device support

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2021/06/04 7:16, Mike Snitzer wrote:
> On Thu, Jun 03 2021 at  1:46P -0400,
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 6/2/21 12:32 PM, Mike Snitzer wrote:
>>> On Tue, Jun 01 2021 at  6:57P -0400,
>>> Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>>>
>>>> On 2021/05/26 6:25, Damien Le Moal wrote:
>>>>> This series improve device mapper support for zoned block devices and
>>>>> of targets exposing a zoned device.
>>>>
>>>> Mike, Jens,
>>>>
>>>> Any feedback regarding this series ?
>>>>
>>>>>
>>>>> The first patch improve support for user requests to reset all zones of
>>>>> the target device. With the fix, such operation behave similarly to
>>>>> physical block devices implementation based on the single zone reset
>>>>> command with the ALL bit set.
>>>>>
>>>>> The following 2 patches are preparatory block layer patches.
>>>>>
>>>>> Patch 4 and 5 are 2 small fixes to DM core zoned block device support.
>>>>>
>>>>> Patch 6 reorganizes DM core code, moving conditionally defined zoned
>>>>> block device code into the new dm-zone.c file. This avoids sprinkly DM
>>>>> with zone related code defined under an #ifdef CONFIG_BLK_DEV_ZONED.
>>>>>
>>>>> Patch 7 improves DM zone report helper functions for target drivers.
>>>>>
>>>>> Patch 8 fixes a potential problem with BIO requeue on zoned target.
>>>>>
>>>>> Finally, patch 9 to 11 implement zone append emulation using regular
>>>>> writes for target drivers that cannot natively support this BIO type.
>>>>> The only target currently needing this emulation is dm-crypt. With this
>>>>> change, a zoned dm-crypt device behaves exactly like a regular zoned
>>>>> block device, correctly executing user zone append BIOs.
>>>>>
>>>>> This series passes the following tests:
>>>>> 1) zonefs tests on top of dm-crypt with a zoned nullblk device
>>>>> 2) zonefs tests on top of dm-crypt+dm-linear with an SMR HDD
>>>>> 3) btrfs fstests on top of dm-crypt with zoned nullblk devices.
>>>>>
>>>>> Comments are as always welcome.
>>>
>>> I've picked up DM patches 4-8 because they didn't depend on the first
>>> 3 block patches.
>>>
>>> But I'm fine with picking up 1-3 if Jens provides his Acked-by.
>>> And then I can pickup the remaining DM patches 9-11.
>>
>> I'm fine with 1-3, you can add my Acked-by to those.
> 
> Thanks, did so.
> 
> Damien: I've staged this patchset in linux-next via the dm-5.14 branch of linux-dm.git

Thanks !

> Might look at optimizing the fast-path of __map_bio further, e.g. this
> leaves something to be desired considering how niche this all is:
> 
>         /*
>          * Check if the IO needs a special mapping due to zone append emulation
>          * on zoned target. In this case, dm_zone_map_bio() calls the target
>          * map operation.
>          */
>         if (dm_emulate_zone_append(io->md))
>                 r = dm_zone_map_bio(tio);
>         else
>                 r = ti->type->map(ti, clone);
> 
> Does it make sense to split out a new CONFIG_ that encapsulates legacy
> zoned devices?

Well, the problem is that there are no "legacy" zoned devices: they all support
zone append commands, either natively (for zns and nullblk) or emulated in their
respective drivers (scsi sd for SMR drives). So I do not think that a new
CONFIG_XXX can be used.

What we could do is have this conditional on either:
(1) the DM targets that need it: dm-crypt only for now (CONFIG_DM_CRYPT)
(2) Zone append command users: btrfs and zonefs only for now (CONFIG_FS_BTRFS
and CONFIG_FS_ZONEFS)

(1) would be the nicest as we can keep this contained in DM and define something
in KConfig. However, given that most distros (if not all) enable dm-crypt, I am
not convinced this will do any good.

Note that for the !CONFIG_BLK_DEV_ZONED case, the "if
(dm_emulate_zone_append(io->md))" is compiled out, resulting in the same code as
without the emulation.

-- 
Damien Le Moal
Western Digital Research



--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel