[dm-devel] [PATCH 0/4] dm: zoned block device fixes

[Damien Le Moal]

Mike,

On 6/9/17 05:21, Mike Snitzer wrote:
> On Mon, Jun 05 2017 at  6:48am -0400,
> Damien Le Moal <damien.lemoal at wdc.com> wrote:
>  
>> I did address some of the "FIXME" notes you added. The main one is the
>> BIO cloning in the I/O path. I removed most of that and added a .end_io
>> method for completion processing. The only place were I do not see how
>> to remove the call to bio_clone() is during read BIO processing: since a
>> read BIO may end up being split between buffer zone, sequential zone and
>> simple buffer zero-out, fragmentation of the read BIO is sometimes
>> necessary and so need a clone.
> 
> So shouldn't it be possible to not allow a given bio to cross zone
> boundaries by using dm_accept_partial_bio()?
> 
> Like you're already doing in dmz_map() actually... so why do you need to
> account for crossing zone boundaries on read later on in
> dmz_submit_read_bio()?

It is not crossing of zone or chunk boundaries that is being delt with
here. When the read BIO is being processed, we already know that it does
not cross chunk boundaries thanks to dmz_map(). Since chunks are mapped
to entire zones, the BIO does not cross a zone boundary either.

But the blocks to read within the chunk may be (1) invalid if they were
never written, (2) valid in the chunk data zone or (3) valid in the
chunk write buffer zone (this case exists only if the chunk is mapped to
a sequential zone.
So we need to examine the zones block bitmaps to discover this and
eventually split the BIO in several fragments if needed. Hence the need
for bio_clone().

If this processing was done within the context of dmz_map(), we could of
course use dm_accept_partial_bio() for splitting the BIO. But I avoided
this method as access to the zone bitmap may trigger metadata I/Os,
which is not very nice in the context of the user bio_submit(). There is
also the fact that dm-zoned does not have per zone locks to deal with
concurrent accesses to the same chunk. All BIO processing for a chunk
get serialized in the chunk work. This is mandatory to ensure sequential
write to sequential zones whenever possible.

> Is it that these zones aren't easily known up front (in dmz_map)?

dmz_map() does the mapping discovery using the mapping table that is
pinned-down in memory. So no metadata I/O trigger. But doing more than
that could trigger metadata I/Os, which I wanted to avoid.

On another note, I just posted an additional small patch for fixing 2
problems: an overflow in the target length calculation and a bad mistake
in the patch with the suspend method setup that caused compilation
error. My apologies for these mistakes. They were not in my local tree
when I tested. I think I messed up with the merging of all my local patches.

I checked your tree again and can confirm now that I am in sync. I will
keep testing anyway to make sure I did not do anything else wrong.

Thank you.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr Manager, System Software Group,
Western Digital Research
Damien.LeMoal at wdc.com
Tel: (+81) 0466-98-3593 (Ext. 51-3593)
1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com