[RFC v2 06/17] bio: do not check bio->bi_status before assigning to it

Andreas Gruenbacher <[email protected]>
Newsgroups gmane.linux.block,gmane.comp.file-systems.btrfs,gmane.linux.raid,gmane.linux.kernel
Message-ID <[email protected]>
In a few places, we check if to->bi_status is 0 before setting it.  This
creates the impression of preserving the first error code that occurs,
but that is not actually the case because bio->bi_status is not updated
atomically.  (And even updating bio->bi_status atomically wouldn't
preserve the first error in bio chains longer than two.)  We don't
actually need to preserve the first error code, though; and all that
matters is that when an error occurs, one of the error codes is
preserved.

Checking bio->bi_status before assigning to it might also have been
meant as an optimization.  We are on an error path and the potential
benefit is miniscule, though.

So, don't check bio->bi_status before assigning to it.

Created with Coccinelle using the following semantic patch:

@@
expression status;
struct bio *bio;
@@
- if (status && !bio->bi_status)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio *bio;
@@
- if (status != BLK_STS_OK && bio->bi_status == BLK_STS_OK)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio *bio;
@@
- if (unlikely(status) && !bio->bi_status)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio bio;
@@
- if (status && !bio.bi_status)
-       bio.bi_status = status;
+ bio_set_status(&bio, status);
---
 block/bio.c                  | 3 +--
 block/fops.c                 | 3 +--
 drivers/md/dm-integrity.c    | 3 +--
 drivers/md/dm-zoned-target.c | 3 +--
 drivers/md/md.c              | 6 ++----
 5 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 3f408e1ba13f..5389321872f0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -314,8 +314,7 @@ static struct bio *__bio_chain_endio(struct bio *bio)
 {
 	struct bio *parent = bio->bi_private;
 
-	if (bio->bi_status && !parent->bi_status)
-		parent->bi_status = bio->bi_status;
+	bio_set_status(parent, bio->bi_status);
 	bio_put(bio);
 	return parent;
 }
diff --git a/block/fops.c b/block/fops.c
index b4f911273289..a4a6972cbfbf 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -135,8 +135,7 @@ static void blkdev_bio_end_io(struct bio *bio)
 	bool should_dirty = dio->flags & DIO_SHOULD_DIRTY;
 	bool is_sync = dio->flags & DIO_IS_SYNC;
 
-	if (bio->bi_status && !dio->bio.bi_status)
-		dio->bio.bi_status = bio->bi_status;
+	bio_set_status(&dio->bio, bio->bi_status);
 
 	if (bio_integrity(bio))
 		bio_integrity_unmap_user(bio);
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index e380ed7b2a7b..6d04b067060d 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -1616,8 +1616,7 @@ static void dec_in_flight(struct dm_integrity_io *dio)
 			schedule_autocommit(ic);
 
 		bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
-		if (unlikely(dio->bi_status) && !bio->bi_status)
-			bio->bi_status = dio->bi_status;
+		bio_set_status(bio, dio->bi_status);
 		if (likely(!bio->bi_status) && unlikely(bio_sectors(bio) != dio->range.n_sectors)) {
 			dio->range.logical_sector += dio->range.n_sectors;
 			bio_advance(bio, dio->range.n_sectors << SECTOR_SHIFT);
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 9da329078ea4..efb960eca8ea 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -77,8 +77,7 @@ static inline void dmz_bio_endio(struct bio *bio, blk_status_t status)
 	struct dmz_bioctx *bioctx =
 		dm_per_bio_data(bio, sizeof(struct dmz_bioctx));
 
-	if (status != BLK_STS_OK && bio->bi_status == BLK_STS_OK)
-		bio->bi_status = status;
+	bio_set_status(bio, status);
 	if (bioctx->dev && bio->bi_status != BLK_STS_OK)
 		bioctx->dev->flags |= DMZ_CHECK_BDEV;
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index cbca792fa380..5afc6d63aa7b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9084,8 +9084,7 @@ static void md_end_clone_io(struct bio *bio)
 	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
 		md_bitmap_end(mddev, md_io_clone);
 
-	if (bio->bi_status && !orig_bio->bi_status)
-		orig_bio->bi_status = bio->bi_status;
+	bio_set_status(orig_bio, bio->bi_status);
 
 	if (md_io_clone->start_time)
 		bio_end_io_acct(orig_bio, md_io_clone->start_time);
@@ -9136,8 +9135,7 @@ void md_free_cloned_bio(struct bio *bio)
 	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
 		md_bitmap_end(mddev, md_io_clone);
 
-	if (bio->bi_status && !orig_bio->bi_status)
-		orig_bio->bi_status = bio->bi_status;
+	bio_set_status(orig_bio, bio->bi_status);
 
 	if (md_io_clone->start_time)
 		bio_end_io_acct(orig_bio, md_io_clone->start_time);
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.