Between the moment when generic_make_request() is first
called on a bio, and when bio_endio() finally gets past
bio_remaining_done(), a bio might have chained children, and might
get ->bi_status set asynchronously.

So during this time it is not safe to set it to zero.
It *is* safe to set it to any other error status as for most
purposes, one error is as good as another.

This patch is untested and RFC only.  It identifies a number of
possible problem areas in dm and often suggests fixes.

Please don't apply without careful review.

Reviewed-by: Nobody yet.
Signed-off-by: NeilBrown <ne...@suse.com>
---
 drivers/md/dm-bio-prison-v1.c | 3 ++-
 drivers/md/dm-bufio.c         | 6 ++++--
 drivers/md/dm-crypt.c         | 3 ++-
 drivers/md/dm-mpath.c         | 3 +++
 drivers/md/dm-thin.c          | 3 ++-
 drivers/md/dm-verity-target.c | 3 ++-
 drivers/md/dm-zoned-target.c  | 3 ++-
 7 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-bio-prison-v1.c b/drivers/md/dm-bio-prison-v1.c
index 874841f0fc83..68cb4fe98e9f 100644
--- a/drivers/md/dm-bio-prison-v1.c
+++ b/drivers/md/dm-bio-prison-v1.c
@@ -238,7 +238,8 @@ void dm_cell_error(struct dm_bio_prison *prison,
        dm_cell_release(prison, cell, &bios);
 
        while ((bio = bio_list_pop(&bios))) {
-               bio->bi_status = error;
+               if (error)
+                       bio->bi_status = error;
                bio_endio(bio);
        }
 }
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 414c9af54ded..80131e098650 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -565,7 +565,8 @@ static void dmio_complete(unsigned long error, void 
*context)
 {
        struct dm_buffer *b = context;
 
-       b->bio.bi_status = error ? BLK_STS_IOERR : 0;
+       if (error)
+               b->bio.bi_status = BLK_STS_IOERR;
        b->bio.bi_end_io(&b->bio);
 }
 
@@ -614,7 +615,8 @@ static void inline_endio(struct bio *bio)
         */
        bio_reset(bio);
 
-       bio->bi_status = status;
+       if (status)
+               bio->bi_status = status;
        end_fn(bio);
 }
 
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8168f737590e..3e7db4a1093e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1488,7 +1488,8 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
        else
                kfree(io->integrity_metadata);
 
-       base_bio->bi_status = error;
+       if (error)
+               base_bio->bi_status = error;
        bio_endio(base_bio);
 }
 
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 7d3e572072f5..6f793d7d29f0 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -651,6 +651,9 @@ static int __multipath_map_bio(struct multipath *m, struct 
bio *bio,
 
        mpio->pgpath = pgpath;
 
+       /* FIXME If the bio was chained, this could
+        * over-write an error... is that possible?
+        */
        bio->bi_status = 0;
        bio_set_dev(bio, pgpath->path.dev->bdev);
        bio->bi_opf |= REQ_FAILFAST_TRANSPORT;
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 629c555890c1..16d4c386cc56 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -563,7 +563,8 @@ static void error_bio_list(struct bio_list *bios, 
blk_status_t error)
        struct bio *bio;
 
        while ((bio = bio_list_pop(bios))) {
-               bio->bi_status = error;
+               if (error)
+                       bio->bi_status = error;
                bio_endio(bio);
        }
 }
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index aedb8222836b..86a1d501d915 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -503,7 +503,8 @@ static void verity_finish_io(struct dm_verity_io *io, 
blk_status_t status)
        struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
 
        bio->bi_end_io = io->orig_bi_end_io;
-       bio->bi_status = status;
+       if (status)
+               bio->bi_status = status;
 
        verity_fec_finish_io(io);
 
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index caff02caf083..cfdacaf79492 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -637,7 +637,8 @@ static int dmz_end_io(struct dm_target *ti, struct bio 
*bio, blk_status_t *error
                return DM_ENDIO_INCOMPLETE;
 
        /* Done */
-       bio->bi_status = bioctx->status;
+       if (bioctx->status)
+               bio->bi_status = bioctx->status;
 
        if (bioctx->zone) {
                struct dm_zone *zone = bioctx->zone;
-- 
2.14.0.rc0.dirty

Attachment: signature.asc
Description: PGP signature

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to