[PATCH] btrfs: Record error in btree_csum_one_bio()

[PATCH] btrfs: Record error in btree_csum_one_bio()

[Qu Wenruo]

Since commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
iterate over multi-page bvec"), break will only break the inner loop of
bio_for_each_segment_all(), unable to break the outer loop.

So here we add a new variable @err to record the first error we hit, and
return the error, so that write time checker can work as designed.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6fe9197f6ee4..9e4d129bb649 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -832,18 +832,18 @@ static blk_status_t btree_csum_one_bio(struct bio *bio)
 {
 	struct bio_vec *bvec;
 	struct btrfs_root *root;
-	int i, ret = 0;
+	int i, ret = 0, err = 0;
 	struct bvec_iter_all iter_all;
 
 	ASSERT(!bio_flagged(bio, BIO_CLONED));
 	bio_for_each_segment_all(bvec, bio, i, iter_all) {
 		root = BTRFS_I(bvec->bv_page->mapping->host)->root;
 		ret = csum_dirty_buffer(root->fs_info, bvec->bv_page);
-		if (ret)
-			break;
+		if (ret && !err)
+			err = ret;
 	}
 
-	return errno_to_blk_status(ret);
+	return errno_to_blk_status(err);
 }
 
 static blk_status_t btree_submit_bio_start(void *private_data, struct bio *bio,
-- 
2.21.0

Re: [PATCH] btrfs: Record error in btree_csum_one_bio()

[Nikolay Borisov]

On 6.04.19 г. 6:13 ч., Qu Wenruo wrote:
> Since commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
> iterate over multi-page bvec"), break will only break the inner loop of
> bio_for_each_segment_all(), unable to break the outer loop.

Breaking of outer loop should be implemented with a goto rather than
introducing a new variable.

> 
> So here we add a new variable @err to record the first error we hit, and
> return the error, so that write time checker can work as designed.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 6fe9197f6ee4..9e4d129bb649 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -832,18 +832,18 @@ static blk_status_t btree_csum_one_bio(struct bio *bio)
>  {
>  	struct bio_vec *bvec;
>  	struct btrfs_root *root;
> -	int i, ret = 0;
> +	int i, ret = 0, err = 0;
>  	struct bvec_iter_all iter_all;
>  
>  	ASSERT(!bio_flagged(bio, BIO_CLONED));
>  	bio_for_each_segment_all(bvec, bio, i, iter_all) {
>  		root = BTRFS_I(bvec->bv_page->mapping->host)->root;
>  		ret = csum_dirty_buffer(root->fs_info, bvec->bv_page);
> -		if (ret)
> -			break;
> +		if (ret && !err)
> +			err = ret;
>  	}
>  
> -	return errno_to_blk_status(ret);
> +	return errno_to_blk_status(err);
>  }
>  
>  static blk_status_t btree_submit_bio_start(void *private_data, struct bio *bio,
> 

[PATCH] btrfs: Record error in btree_csum_one_bio()

[Qu Wenruo]

Since commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
iterate over multi-page bvec"), break will only break the inner loop of
bio_for_each_segment_all(), unable to break the outer loop.

This patch will break out by utilizing goto command.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6fe9197f6ee4..e20b952d3cde 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -840,9 +840,10 @@ static blk_status_t btree_csum_one_bio(struct bio *bio)
 		root = BTRFS_I(bvec->bv_page->mapping->host)->root;
 		ret = csum_dirty_buffer(root->fs_info, bvec->bv_page);
 		if (ret)
-			break;
+			goto out;
 	}
 
+out:
 	return errno_to_blk_status(ret);
 }
 
-- 
2.21.0

Re: [PATCH] btrfs: Record error in btree_csum_one_bio()

[David Sterba]

On Sat, Apr 06, 2019 at 06:20:05PM +0800, Qu Wenruo wrote:
> Since commit 6dc4f100c175 ("block: allow bio_for_each_segment_all() to
> iterate over multi-page bvec"), break will only break the inner loop of
> bio_for_each_segment_all(), unable to break the outer loop.
> 
> This patch will break out by utilizing goto command.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/disk-io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 6fe9197f6ee4..e20b952d3cde 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -840,9 +840,10 @@ static blk_status_t btree_csum_one_bio(struct bio *bio)
>  		root = BTRFS_I(bvec->bv_page->mapping->host)->root;
>  		ret = csum_dirty_buffer(root->fs_info, bvec->bv_page);
>  		if (ret)
> -			break;
> +			goto out;

This being fixed in the block layer macros, but note that 'break' is not
the only thing that's broken, also 'continue' and we do have a few
instances of that in btrfs code.

Once this is fixed in the macros, this patch is not necessary.