All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Btrfs: fix unexpected return value of bio_readpage_error
@ 2017-07-13 22:00 Liu Bo
  2017-07-14  7:04 ` Nikolay Borisov
  2017-07-14 12:32 ` David Sterba
  0 siblings, 2 replies; 4+ messages in thread
From: Liu Bo @ 2017-07-13 22:00 UTC (permalink / raw)
  To: linux-btrfs

With blk_status_t, in bio_readpage_error() may return 1 as now
->submit_bio_hook() may not set %ret if it runs without problem.

This fixes that unexpected return value by changing
btrfs_check_repairable() to return a bool instead of updating %ret.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/extent_io.c | 12 ++++++------
 fs/btrfs/extent_io.h |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 556484c..098c319 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2258,7 +2258,7 @@ int btrfs_get_io_failure_record(struct inode *inode, u64 start, u64 end,
 	return 0;
 }
 
-int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
+bool btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
 			   struct io_failure_record *failrec, int failed_mirror)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
@@ -2274,7 +2274,7 @@ int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
 		btrfs_debug(fs_info,
 			"Check Repairable: cannot repair, num_copies=%d, next_mirror %d, failed_mirror %d",
 			num_copies, failrec->this_mirror, failed_mirror);
-		return 0;
+		return false;
 	}
 
 	/*
@@ -2315,10 +2315,10 @@ int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
 		btrfs_debug(fs_info,
 			"Check Repairable: (fail) num_copies=%d, next_mirror %d, failed_mirror %d",
 			num_copies, failrec->this_mirror, failed_mirror);
-		return 0;
+		return false;
 	}
 
-	return 1;
+	return true;
 }
 
 
@@ -2382,8 +2382,8 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset,
 	if (ret)
 		return ret;
 
-	ret = btrfs_check_repairable(inode, failed_bio, failrec, failed_mirror);
-	if (!ret) {
+	if (!btrfs_check_repairable(inode, failed_bio, failrec,
+				    failed_mirror)) {
 		free_io_failure(failure_tree, tree, failrec);
 		return -EIO;
 	}
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 3fb8513..4f03091 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -539,8 +539,8 @@ void btrfs_free_io_failure_record(struct btrfs_inode *inode, u64 start,
 		u64 end);
 int btrfs_get_io_failure_record(struct inode *inode, u64 start, u64 end,
 				struct io_failure_record **failrec_ret);
-int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
-			   struct io_failure_record *failrec, int fail_mirror);
+bool btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
+			    struct io_failure_record *failrec, int fail_mirror);
 struct bio *btrfs_create_repair_bio(struct inode *inode, struct bio *failed_bio,
 				    struct io_failure_record *failrec,
 				    struct page *page, int pg_offset, int icsum,
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Btrfs: fix unexpected return value of bio_readpage_error
  2017-07-13 22:00 [PATCH] Btrfs: fix unexpected return value of bio_readpage_error Liu Bo
@ 2017-07-14  7:04 ` Nikolay Borisov
  2017-07-14 12:32 ` David Sterba
  1 sibling, 0 replies; 4+ messages in thread
From: Nikolay Borisov @ 2017-07-14  7:04 UTC (permalink / raw)
  To: Liu Bo, linux-btrfs



On 14.07.2017 01:00, Liu Bo wrote:
> With blk_status_t, in bio_readpage_error() may return 1 as now
> ->submit_bio_hook() may not set %ret if it runs without problem.
> 
> This fixes that unexpected return value by changing
> btrfs_check_repairable() to return a bool instead of updating %ret.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/extent_io.c | 12 ++++++------
>  fs/btrfs/extent_io.h |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 556484c..098c319 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2258,7 +2258,7 @@ int btrfs_get_io_failure_record(struct inode *inode, u64 start, u64 end,
>  	return 0;
>  }
>  
> -int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
> +bool btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
>  			   struct io_failure_record *failrec, int failed_mirror)
>  {
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> @@ -2274,7 +2274,7 @@ int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
>  		btrfs_debug(fs_info,
>  			"Check Repairable: cannot repair, num_copies=%d, next_mirror %d, failed_mirror %d",
>  			num_copies, failrec->this_mirror, failed_mirror);
> -		return 0;
> +		return false;
>  	}
>  
>  	/*
> @@ -2315,10 +2315,10 @@ int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
>  		btrfs_debug(fs_info,
>  			"Check Repairable: (fail) num_copies=%d, next_mirror %d, failed_mirror %d",
>  			num_copies, failrec->this_mirror, failed_mirror);
> -		return 0;
> +		return false;
>  	}
>  
> -	return 1;
> +	return true;
>  }
>  
>  
> @@ -2382,8 +2382,8 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset,
>  	if (ret)
>  		return ret;
>  
> -	ret = btrfs_check_repairable(inode, failed_bio, failrec, failed_mirror);
> -	if (!ret) {
> +	if (!btrfs_check_repairable(inode, failed_bio, failrec,
> +				    failed_mirror)) {
>  		free_io_failure(failure_tree, tree, failrec);
>  		return -EIO;
>  	}
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 3fb8513..4f03091 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -539,8 +539,8 @@ void btrfs_free_io_failure_record(struct btrfs_inode *inode, u64 start,
>  		u64 end);
>  int btrfs_get_io_failure_record(struct inode *inode, u64 start, u64 end,
>  				struct io_failure_record **failrec_ret);
> -int btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
> -			   struct io_failure_record *failrec, int fail_mirror);
> +bool btrfs_check_repairable(struct inode *inode, struct bio *failed_bio,
> +			    struct io_failure_record *failrec, int fail_mirror);
>  struct bio *btrfs_create_repair_bio(struct inode *inode, struct bio *failed_bio,
>  				    struct io_failure_record *failrec,
>  				    struct page *page, int pg_offset, int icsum,
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Btrfs: fix unexpected return value of bio_readpage_error
  2017-07-13 22:00 [PATCH] Btrfs: fix unexpected return value of bio_readpage_error Liu Bo
  2017-07-14  7:04 ` Nikolay Borisov
@ 2017-07-14 12:32 ` David Sterba
  2017-07-14 16:30   ` Liu Bo
  1 sibling, 1 reply; 4+ messages in thread
From: David Sterba @ 2017-07-14 12:32 UTC (permalink / raw)
  To: Liu Bo; +Cc: linux-btrfs

On Thu, Jul 13, 2017 at 03:00:50PM -0700, Liu Bo wrote:
> With blk_status_t, in bio_readpage_error() may return 1 as now
> ->submit_bio_hook() may not set %ret if it runs without problem.

Huh, that is pretty subtle, good catch. Also it's good the patch can be
applied independent of the blk_status_t changes, so I don't need to do 2
branches and pull requests.

> This fixes that unexpected return value by changing
> btrfs_check_repairable() to return a bool instead of updating %ret.
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Btrfs: fix unexpected return value of bio_readpage_error
  2017-07-14 12:32 ` David Sterba
@ 2017-07-14 16:30   ` Liu Bo
  0 siblings, 0 replies; 4+ messages in thread
From: Liu Bo @ 2017-07-14 16:30 UTC (permalink / raw)
  To: dsterba, linux-btrfs

On Fri, Jul 14, 2017 at 02:32:09PM +0200, David Sterba wrote:
> On Thu, Jul 13, 2017 at 03:00:50PM -0700, Liu Bo wrote:
> > With blk_status_t, in bio_readpage_error() may return 1 as now
> > ->submit_bio_hook() may not set %ret if it runs without problem.
> 
> Huh, that is pretty subtle, good catch. Also it's good the patch can be
> applied independent of the blk_status_t changes, so I don't need to do 2
> branches and pull requests.
>

Thanks to the ASSERT(ret == -EIO) in end_bio_extent_readpage() ;)

thanks,
-liubo

> > This fixes that unexpected return value by changing
> > btrfs_check_repairable() to return a bool instead of updating %ret.
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> 
> Reviewed-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-14 16:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-13 22:00 [PATCH] Btrfs: fix unexpected return value of bio_readpage_error Liu Bo
2017-07-14  7:04 ` Nikolay Borisov
2017-07-14 12:32 ` David Sterba
2017-07-14 16:30   ` Liu Bo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.