[PATCH] btrfs: let check_async_write return bool

[PATCH] btrfs: let check_async_write return bool

[Johannes Thumshirn]

The 'check_async_write' function is a helper used in
'btrfs_submit_metadata_bio' and it checks if asynchronous writing can be
used for metadata.

Make the function return bool and get rid of the local variable async in
btrfs_submit_metadata_bio storing the result of check_async_write's tests.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/disk-io.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c9a3036c23bf..4305e3a26a6b 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -917,23 +917,22 @@ static blk_status_t btree_submit_bio_start(struct inode *inode, struct bio *bio,
 	return btree_csum_one_bio(bio);
 }
 
-static int check_async_write(struct btrfs_fs_info *fs_info,
+static bool check_async_write(struct btrfs_fs_info *fs_info,
 			     struct btrfs_inode *bi)
 {
 	if (btrfs_is_zoned(fs_info))
-		return 0;
+		return false;
 	if (atomic_read(&bi->sync_writers))
-		return 0;
+		return false;
 	if (test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
-		return 0;
-	return 1;
+		return false;
+	return true;
 }
 
 blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
 				       int mirror_num, unsigned long bio_flags)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-	int async = check_async_write(fs_info, BTRFS_I(inode));
 	blk_status_t ret;
 
 	if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
@@ -946,7 +945,7 @@ blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
 		if (ret)
 			goto out_w_error;
 		ret = btrfs_map_bio(fs_info, bio, mirror_num);
-	} else if (!async) {
+	} else if (!check_async_write(fs_info, BTRFS_I(inode))) {
 		ret = btree_csum_one_bio(bio);
 		if (ret)
 			goto out_w_error;
-- 
2.31.1

Re: [PATCH] btrfs: let check_async_write return bool

[Nikolay Borisov]

On 10.05.21 г. 18:17, Johannes Thumshirn wrote:
> The 'check_async_write' function is a helper used in
> 'btrfs_submit_metadata_bio' and it checks if asynchronous writing can be
> used for metadata.
> 
> Make the function return bool and get rid of the local variable async in
> btrfs_submit_metadata_bio storing the result of check_async_write's tests.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Nit: Won't it make it more clear if the function is named
"should_write_async" or something along those lines?

> ---
>  fs/btrfs/disk-io.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c9a3036c23bf..4305e3a26a6b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -917,23 +917,22 @@ static blk_status_t btree_submit_bio_start(struct inode *inode, struct bio *bio,
>  	return btree_csum_one_bio(bio);
>  }
>  
> -static int check_async_write(struct btrfs_fs_info *fs_info,
> +static bool check_async_write(struct btrfs_fs_info *fs_info,
>  			     struct btrfs_inode *bi)
>  {
>  	if (btrfs_is_zoned(fs_info))
> -		return 0;
> +		return false;
>  	if (atomic_read(&bi->sync_writers))
> -		return 0;
> +		return false;
>  	if (test_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags))
> -		return 0;
> -	return 1;
> +		return false;
> +	return true;
>  }
>  
>  blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
>  				       int mirror_num, unsigned long bio_flags)
>  {
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> -	int async = check_async_write(fs_info, BTRFS_I(inode));
>  	blk_status_t ret;
>  
>  	if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
> @@ -946,7 +945,7 @@ blk_status_t btrfs_submit_metadata_bio(struct inode *inode, struct bio *bio,
>  		if (ret)
>  			goto out_w_error;
>  		ret = btrfs_map_bio(fs_info, bio, mirror_num);
> -	} else if (!async) {
> +	} else if (!check_async_write(fs_info, BTRFS_I(inode))) {
>  		ret = btree_csum_one_bio(bio);
>  		if (ret)
>  			goto out_w_error;
> 

Re: [PATCH] btrfs: let check_async_write return bool

[David Sterba]

On Mon, May 10, 2021 at 07:29:39PM +0300, Nikolay Borisov wrote:
> 
> 
> On 10.05.21 г. 18:17, Johannes Thumshirn wrote:
> > The 'check_async_write' function is a helper used in
> > 'btrfs_submit_metadata_bio' and it checks if asynchronous writing can be
> > used for metadata.
> > 
> > Make the function return bool and get rid of the local variable async in
> > btrfs_submit_metadata_bio storing the result of check_async_write's tests.
> > 
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> Nit: Won't it make it more clear if the function is named
> "should_write_async" or something along those lines?

Yeah that sounds better and more consistent with other check_* or
should_* helpers.

Re: [PATCH] btrfs: let check_async_write return bool

[David Sterba]

On Tue, May 11, 2021 at 12:17:26AM +0900, Johannes Thumshirn wrote:
> The 'check_async_write' function is a helper used in
> 'btrfs_submit_metadata_bio' and it checks if asynchronous writing can be
> used for metadata.
> 
> Make the function return bool and get rid of the local variable async in
> btrfs_submit_metadata_bio storing the result of check_async_write's tests.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

With the suggested name applied to misc-next, thanks.