[PATCH] btrfs: reuse exit helper in btrfs_bioset_init()

[PATCH] btrfs: reuse exit helper in btrfs_bioset_init()

[Yangtao Li]

As David Sterba said before:

  This is partially duplicating btrfs_delayed_ref_exit(), I'd rather reuse
  the exit helper.

  I've checked if this can be done elsewhere, seems that there's only one
  other case btrfs_bioset_init(), which is partially duplicating
  btrfs_bioset_exit(). All other init/exit functions are trivial and
  allocate one structure. So if you want to do that cleanup, please update
  btrfs_bioset_init() to the preferred pattern. Thanks.

So let's convert it.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/btrfs/bio.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 8c2eee1f1878..f6f84837d62b 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -892,6 +892,14 @@ void btrfs_submit_repair_write(struct btrfs_bio *bbio, int mirror_num, bool dev_
 	btrfs_bio_end_io(bbio, errno_to_blk_status(ret));
 }
 
+void __cold btrfs_bioset_exit(void)
+{
+	mempool_exit(&btrfs_failed_bio_pool);
+	bioset_exit(&btrfs_repair_bioset);
+	bioset_exit(&btrfs_clone_bioset);
+	bioset_exit(&btrfs_bioset);
+}
+
 int __init btrfs_bioset_init(void)
 {
 	if (bioset_init(&btrfs_bioset, BIO_POOL_SIZE,
@@ -900,29 +908,17 @@ int __init btrfs_bioset_init(void)
 		return -ENOMEM;
 	if (bioset_init(&btrfs_clone_bioset, BIO_POOL_SIZE,
 			offsetof(struct btrfs_bio, bio), 0))
-		goto out_free_bioset;
+		goto out;
 	if (bioset_init(&btrfs_repair_bioset, BIO_POOL_SIZE,
 			offsetof(struct btrfs_bio, bio),
 			BIOSET_NEED_BVECS))
-		goto out_free_clone_bioset;
+		goto out;
 	if (mempool_init_kmalloc_pool(&btrfs_failed_bio_pool, BIO_POOL_SIZE,
 				      sizeof(struct btrfs_failed_bio)))
-		goto out_free_repair_bioset;
+		goto out;
 	return 0;
 
-out_free_repair_bioset:
-	bioset_exit(&btrfs_repair_bioset);
-out_free_clone_bioset:
-	bioset_exit(&btrfs_clone_bioset);
-out_free_bioset:
-	bioset_exit(&btrfs_bioset);
+out:
+	btrfs_bioset_exit();
 	return -ENOMEM;
 }
-
-void __cold btrfs_bioset_exit(void)
-{
-	mempool_exit(&btrfs_failed_bio_pool);
-	bioset_exit(&btrfs_repair_bioset);
-	bioset_exit(&btrfs_clone_bioset);
-	bioset_exit(&btrfs_bioset);
-}
-- 
2.39.0

Re: [PATCH] btrfs: reuse exit helper in btrfs_bioset_init()

[David Sterba]

On Mon, Apr 14, 2025 at 06:44:01AM -0600, Yangtao Li wrote:
> As David Sterba said before:
> 
>   This is partially duplicating btrfs_delayed_ref_exit(), I'd rather reuse
>   the exit helper.
> 
>   I've checked if this can be done elsewhere, seems that there's only one
>   other case btrfs_bioset_init(), which is partially duplicating
>   btrfs_bioset_exit(). All other init/exit functions are trivial and
>   allocate one structure. So if you want to do that cleanup, please update
>   btrfs_bioset_init() to the preferred pattern. Thanks.

Please write the changelogs as standalone text without the references or
copied text from some suggestions.

Mentions, credits or Suggested-by make most sense if there's some
groundbreaking idea implemented and not mentioning the author would be
percieved as stealing it. Otherwise, suggestions are part of the
review process and should be transformed into useful text in the
changelog.

> So let's convert it.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/btrfs/bio.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 8c2eee1f1878..f6f84837d62b 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -892,6 +892,14 @@ void btrfs_submit_repair_write(struct btrfs_bio *bbio, int mirror_num, bool dev_
>  	btrfs_bio_end_io(bbio, errno_to_blk_status(ret));
>  }
>  
> +void __cold btrfs_bioset_exit(void)
> +{
> +	mempool_exit(&btrfs_failed_bio_pool);
> +	bioset_exit(&btrfs_repair_bioset);
> +	bioset_exit(&btrfs_clone_bioset);
> +	bioset_exit(&btrfs_bioset);
> +}

This function is public and you don't need to move it.

> +
>  int __init btrfs_bioset_init(void)
>  {
>  	if (bioset_init(&btrfs_bioset, BIO_POOL_SIZE,
> @@ -900,29 +908,17 @@ int __init btrfs_bioset_init(void)
>  		return -ENOMEM;
>  	if (bioset_init(&btrfs_clone_bioset, BIO_POOL_SIZE,
>  			offsetof(struct btrfs_bio, bio), 0))
> -		goto out_free_bioset;
> +		goto out;
>  	if (bioset_init(&btrfs_repair_bioset, BIO_POOL_SIZE,
>  			offsetof(struct btrfs_bio, bio),
>  			BIOSET_NEED_BVECS))
> -		goto out_free_clone_bioset;
> +		goto out;
>  	if (mempool_init_kmalloc_pool(&btrfs_failed_bio_pool, BIO_POOL_SIZE,
>  				      sizeof(struct btrfs_failed_bio)))
> -		goto out_free_repair_bioset;
> +		goto out;
>  	return 0;
>  
> -out_free_repair_bioset:
> -	bioset_exit(&btrfs_repair_bioset);
> -out_free_clone_bioset:
> -	bioset_exit(&btrfs_clone_bioset);
> -out_free_bioset:
> -	bioset_exit(&btrfs_bioset);
> +out:
> +	btrfs_bioset_exit();
>  	return -ENOMEM;
>  }
> -
> -void __cold btrfs_bioset_exit(void)
> -{
> -	mempool_exit(&btrfs_failed_bio_pool);
> -	bioset_exit(&btrfs_repair_bioset);
> -	bioset_exit(&btrfs_clone_bioset);
> -	bioset_exit(&btrfs_bioset);
> -}
> -- 
> 2.39.0
>