Mike, On 6/9/17 05:21, Mike Snitzer wrote: > On Mon, Jun 05 2017 at 6:48am -0400, > Damien Le Moal <damien.lemoal@xxxxxxx> 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@xxxxxxx Tel: (+81) 0466-98-3593 (Ext. 51-3593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.wdc.com, www.hgst.com -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
