FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 04-27-2011, 10:02 AM
Christoph Hellwig
 
Default dm-thinp: fix REQ_FLUSH semantics

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>

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
 
Old 04-27-2011, 10:09 AM
Joe Thornber
 
Default dm-thinp: fix REQ_FLUSH semantics

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
 
Old 04-27-2011, 12:20 PM
Christoph Hellwig
 
Default dm-thinp: fix REQ_FLUSH semantics

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
 

Thread Tools




All times are GMT. The time now is 12:06 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org