btrfs: handle bio_split() error
Subsystems (2)
PATCH v2 1/1 btrfs: handle bio_split() error
From: Johannes Thumshirn <[email protected]>
Now that bio_split() can return errors, add error handling for it in btrfs_split_bio() and ultimately btrfs_submit_chunk().
---
This is based on top of John Garry's series "bio_split() error handling rework" explicitly on the patch titled "block: Rework bio_split() return value", which are as of now (Tue Oct 29 10:02:16 2024) not yet merged into any tree.
Changes to v1:
- convert ERR_PTR to blk_status_t
- correctly fail already split bbios
--- fs/btrfs/bio.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
fs/btrfs/bio.c +12 -4
@@ -81,6 +81,9 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info, bio =bio_split (&orig_bbio->bio, map_length >>SECTOR_SHIFT ,GFP_NOFS , &btrfs_clone_bioset); +if (IS_ERR (bio)) +return ERR_CAST (bio); + bbio =btrfs_bio (bio);btrfs_bio_init (bbio, fs_info,NULL , orig_bbio); bbio->inode = orig_bbio->inode;
@@ -687,6 +690,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)if (map_length < length) { bbio =btrfs_split_bio (fs_info, bbio, map_length); +if (IS_ERR (bbio)) { + ret =errno_to_blk_status (PTR_ERR (bbio)); + goto fail; + } bio = &bbio->bio; }
@@ -698,7 +705,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) bbio->saved_iter = bio->bi_iter; ret =btrfs_lookup_bio_sums (bbio);if (ret) - goto fail; + goto fail_split; }if (btrfs_op (bio) ==BTRFS_MAP_WRITE ) {
@@ -732,13 +739,13 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) ret =btrfs_bio_csum (bbio);if (ret) - goto fail; + goto fail_split; }else if (use_append || (btrfs_is_zoned (fs_info) && inode && inode->flags &BTRFS_INODE_NODATASUM )) { ret =btrfs_alloc_dummy_sum (bbio);if (ret) - goto fail; + goto fail_split; } }
@@ -746,7 +753,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) done:return map_length == length; -fail: +fail_split:btrfs_bio_counter_dec (fs_info);/* * We have split the original bbio, now we have to end both the current
@@ -760,6 +767,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)btrfs_bio_end_io (remaining, ret); } +fail:btrfs_bio_end_io (bbio, ret);/* Do not submit another chunk */ return true;
Signature · 1 line
2.43.0
Quoted 35 lines · Johannes Thumshirn
From: Johannes Thumshirn <[email protected]>
Now that
bio_split()can return errors, add error handling for it inbtrfs_split_bio()and ultimatelybtrfs_submit_chunk().Signed-off-by Johannes Thumshirn <[email protected]>---
This is based on top of John Garry's series "
bio_split()error handling rework" explicitly on the patch titled "block: Reworkbio_split()return value", which are as of now (Tue Oct 29 10:02:16 2024) not yet merged into any tree.Changes to v1:
- convertERR_PTRtoblk_status_t
- correctly fail already split bbios--- fs/btrfs/bio.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)fs/btrfs/bio.c +3
-0@@ -81,6 +81,9 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info, bio =bio_split (&orig_bbio->bio, map_length >>SECTOR_SHIFT ,GFP_NOFS , &btrfs_clone_bioset); +if (IS_ERR (bio)) +return ERR_CAST (bio); + bbio =btrfs_bio (bio);btrfs_bio_init (bbio, fs_info,NULL , orig_bbio); bbio->inode = orig_bbio->inode;
fs/btrfs/bio.c static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) · line 694
Quoted 5 lines
fs/btrfs/bio.c +2
-0@@ -687,6 +690,10 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)if (map_length < length) { bbio =btrfs_split_bio (fs_info, bbio, map_length); +if (IS_ERR (bbio)) { + ret =errno_to_blk_status (PTR_ERR (bbio));
btrfs_split_bio() does splitting, but error means "goto fail". However other failure points "goto fail_split". I find that label naming odd.
Furthermore, at "fail", we call btrfs_bio_end_io() and that function dereferences bbio (which is -EINVAL or something like that)
Thanks,
John
Quoted 47 lines
fs/btrfs/bio.c +7
-4+ goto fail; + } bio = &bbio->bio; }@@ -698,7 +705,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) bbio->saved_iter = bio->bi_iter; ret =btrfs_lookup_bio_sums (bbio);if (ret) - goto fail; + goto fail_split; }if (btrfs_op (bio) ==BTRFS_MAP_WRITE ) {@@ -732,13 +739,13 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) ret =btrfs_bio_csum (bbio);if (ret) - goto fail; + goto fail_split; }else if (use_append || (btrfs_is_zoned (fs_info) && inode && inode->flags &BTRFS_INODE_NODATASUM )) { ret =btrfs_alloc_dummy_sum (bbio);if (ret) - goto fail; + goto fail_split; } }@@ -746,7 +753,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) done:return map_length == length; -fail: +fail_split:btrfs_bio_counter_dec (fs_info);/* * We have split the original bbio, now we have to end both the current @@ -760,6 +767,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)btrfs_bio_end_io (remaining, ret); } +fail:btrfs_bio_end_io (bbio, ret);/* Do not submit another chunk */ return true;
Quoted 11 lines · John Garry
fs/btrfs/bio.c static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num) · line 694
Quoted 5 lines
if (map_length < length) { bbio = btrfs_split_bio(fs_info, bbio, map_length);fs/btrfs/bio.c +2
-0+if (IS_ERR (bbio)) { + ret =errno_to_blk_status (PTR_ERR (bbio));
btrfs_split_bio()does splitting, but error means "goto fail". However other failure points "gotofail_split". I find that label naming odd.Furthermore, at "fail", we call
btrfs_bio_end_io()and that function dereferences bbio (which is-EINVALor something like that)
OK, my stupidity doing something that should be as simple as this patch is getting embarrassing.
I'll send a v3 soon.