[dm-devel] Re: [PATCH 12/17] bottom-layer barrier support

[Mikulas Patocka]

On Mon, 27 Apr 2009, Alasdair G Kergon wrote:

> On Mon, Apr 20, 2009 at 04:01:11AM -0400, Mikulas Patocka wrote:
> > Flush supoprt for dm-delay target.
>  
> > @@ -281,6 +282,8 @@ static int delay_map(struct dm_target *t
> >  		bio->bi_bdev = dc->dev_write->bdev;
> >  		bio->bi_sector = dc->start_write +
> >  				 (bio->bi_sector - ti->begin);
> > +		if (!bio_sectors(bio))
> > +			bio->bi_sector = 0;
> 
> Why set bio->bi_sector twice on that path?
> Should we instead guarantee it's 0 before calling?

For non-barrier requests, it needs to be translated according to the 
table.

For zero-barrier request, it should be 0. (well, non-zero values may work, 
but I'm setting it zero, all other kernel code sets it to zero too).

> >  
> >  		return delay_bio(dc, dc->write_delay, bio);
> >  	}
> 
> Is flushing different - do we really want to delay it?
> (I'd have thought not.  But if so, should it be a separate option?)

Depending how you want it. I'd see it as a write and delay it as a write. 
If dm-delay is to be used for example for debugging, it should have a 
capability to delay all requests, including flushes.

> Alasdair
> -- 
> agk at redhat.com

Mikulas