Re: [PATCH 5/8] dm: always setup ->orig_bio in alloc_io

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 4/13/22 05:52, Mike Snitzer wrote:
> On Tue, Apr 12 2022 at  4:56P -0400,
> Ming Lei <ming.lei@redhat.com> wrote:
> 
>> The current DM codes setup ->orig_bio after __map_bio() returns,
>> and not only cause kernel panic for dm zone, but also a bit ugly
>> and tricky, especially the waiting until ->orig_bio is set in
>> dm_submit_bio_remap().
>>
>> The reason is that one new bio is cloned from original FS bio to
>> represent the mapped part, which just serves io accounting.
>>
>> Now we have switched to bdev based io accounting interface, and we
>> can retrieve sectors/bio_op from both the real original bio and the
>> added fields of .sector_offset & .sectors easily, so the new cloned
>> bio isn't necessary any more.
>>
>> Not only fixes dm-zone's kernel panic, but also cleans up dm io
>> accounting & split a bit.
> 
> You're conflating quite a few things here.  DM zone really has no
> business accessing io->orig_bio (dm-zone.c can just as easily inspect
> the tio->clone, because it hasn't been remapped yet it reflects the
> io->origin_bio, so there is no need to look at io->orig_bio) -- but
> yes I clearly broke things during the 5.18 merge and it needs fixing
> ASAP.

Problem is that we need to look at the BIO op in submission AND completion
path to handle zone append requests. So looking at the clone on submission
is OK since its op is still the original one. But on the completion path,
the clone may now be a regular write emulating a zone append op. And
looking at the clone only does not allow detecting that zone append.

We could have the orig_bio op saved in dm_io to avoid completely looking
at the orig_bio for detecting zone append.

> 
> But I'm (ab)using io->orig_bio assignment to indicate to completion
> that it may proceed.  See these dm-5.19 commits to see it imposed even
> further:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=311a8e6650601a79079000466db77386c5ec2abb
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=56219ebb5f5c84785aa821f755d545eae41bdb1a
> 
> And then leveraged here:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.19&id=4aa7a368370c2a172d5a0b8927c6332c4b6a3514
> 
> Could be all these dm-5.19 changes suck.. but I do know dm-zone.c is
> too tightly coupled to DM core.  So I'll focus on that first, fix
> 5.18, and then circle back to "what's next?".
> 
> Mike
> 


-- 
Damien Le Moal
Western Digital Research