[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87h8qipqxg.fsf@notabene.neil.brown.name>
Date: Thu, 15 Feb 2018 20:01:31 +1100
From: NeilBrown <neilb@...e.com>
To: Milan Broz <gmazyland@...il.com>,
Mike Snitzer <snitzer@...hat.com>,
device-mapper development <dm-devel@...hat.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: [RFC PATCH] dm: don't assign zero to ->bi_status of an active bio.
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 <neilb@...e.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
Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)
Powered by blists - more mailing lists