| Message ID | 20250414124401.739723-1-frank.li@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: reuse exit helper in btrfs_bioset_init() | expand |
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 >
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); -}
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(-)