| Message ID | 20110427100246.GA22521@infradead.org (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Wed, 2011-04-27 at 06:02 -0400, Christoph Hellwig wrote: > REQ_FLUSH means the metadata needs to be flushed before the data payload > (if there is one), not after it. And yes, this means the typical > REQ_FUA|REQ_FLUSH requests imply two flushes. Not sure what you're getting at here. The bio wasn't issued until after the commit. if ((bio->bi_rw & (REQ_FUA | REQ_FLUSH))) { r = commit(tc); if (r < 0) { bio_io_error(bio); continue; } } remap_bio(tc, bio, pool_block); generic_make_request(bio); - Joe > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: linux-2.6/drivers/md/dm-thin-prov.c > =================================================================== > --- linux-2.6.orig/drivers/md/dm-thin-prov.c 2011-04-27 11:45:50.957995412 +0200 > +++ linux-2.6/drivers/md/dm-thin-prov.c 2011-04-27 11:45:56.954629592 +0200 > @@ -229,6 +229,14 @@ static void do_bios(struct thinp_c *tc, > block_t thinp_block, pool_block; > > while ((bio = bio_list_pop(bios))) { > + if (bio->bi_rw & REQ_FLUSH) { > + r = commit(tc); > + if (r < 0) { > + bio_io_error(bio); > + continue; > + } > + } > + > thinp_block = _sector_to_block(tc, bio->bi_sector); > r = thinp_metadata_lookup(tc->tpm, thinp_block, 1, &pool_block); > if (r == -ENODATA) { > @@ -258,7 +266,7 @@ static void do_bios(struct thinp_c *tc, > * whether the overhead of tracking pending blocks > * is worth it though. > */ > - if ((bio->bi_rw & (REQ_FUA | REQ_FLUSH))) { > + if (bio->bi_rw & REQ_FUA) { > r = commit(tc); > if (r < 0) { > bio_io_error(bio); -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Apr 27, 2011 at 11:09:16AM +0100, Joe Thornber wrote: > On Wed, 2011-04-27 at 06:02 -0400, Christoph Hellwig wrote: > > REQ_FLUSH means the metadata needs to be flushed before the data payload > > (if there is one), not after it. And yes, this means the typical > > REQ_FUA|REQ_FLUSH requests imply two flushes. > > Not sure what you're getting at here. The bio wasn't issued until after > the commit. > > if ((bio->bi_rw & (REQ_FUA | REQ_FLUSH))) { > r = commit(tc); > if (r < 0) { > bio_io_error(bio); > continue; > } > } > > remap_bio(tc, bio, pool_block); > generic_make_request(bio); Indeed. Given that the metadata is on-disk and doesn't change by doing I/O to the newly mapped block always doing it before the I/O to the underlying device is fine. I thus retract this patch. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Index: linux-2.6/drivers/md/dm-thin-prov.c =================================================================== --- linux-2.6.orig/drivers/md/dm-thin-prov.c 2011-04-27 11:45:50.957995412 +0200 +++ linux-2.6/drivers/md/dm-thin-prov.c 2011-04-27 11:45:56.954629592 +0200 @@ -229,6 +229,14 @@ static void do_bios(struct thinp_c *tc, block_t thinp_block, pool_block; while ((bio = bio_list_pop(bios))) { + if (bio->bi_rw & REQ_FLUSH) { + r = commit(tc); + if (r < 0) { + bio_io_error(bio); + continue; + } + } + thinp_block = _sector_to_block(tc, bio->bi_sector); r = thinp_metadata_lookup(tc->tpm, thinp_block, 1, &pool_block); if (r == -ENODATA) { @@ -258,7 +266,7 @@ static void do_bios(struct thinp_c *tc, * whether the overhead of tracking pending blocks * is worth it though. */ - if ((bio->bi_rw & (REQ_FUA | REQ_FLUSH))) { + if (bio->bi_rw & REQ_FUA) { r = commit(tc); if (r < 0) { bio_io_error(bio);
REQ_FLUSH means the metadata needs to be flushed before the data payload (if there is one), not after it. And yes, this means the typical REQ_FUA|REQ_FLUSH requests imply two flushes. Signed-off-by: Christoph Hellwig <hch@lst.de> -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel