btrfs: raid56: use kzalloc_flex for rbio

[PATCH] btrfs: raid56: use kzalloc_flex for rbio

Posted by Rosen Penev 1 month ago

Simplifies allocation slightly. Now stripe_pages does not need to be
freed separately.

Added __counted_by for extra runtime analysis.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 fs/btrfs/raid56.c |  9 ++++-----
 fs/btrfs/raid56.h | 12 ++++++------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index baebd9f733e9..d74ae380265e 100644
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -151,7 +151,6 @@ static void free_raid_bio_pointers(struct btrfs_raid_bio *rbio)
 {
 	bitmap_free(rbio->error_bitmap);
 	bitmap_free(rbio->stripe_uptodate_bitmap);
-	kfree(rbio->stripe_pages);
 	kfree(rbio->bio_paddrs);
 	kfree(rbio->stripe_paddrs);
 	kfree(rbio->finish_pointers);
@@ -1070,10 +1069,11 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
 	ASSERT(real_stripes >= 2);
 	ASSERT(real_stripes <= U8_MAX);
 
-	rbio = kzalloc_obj(*rbio, GFP_NOFS);
+	rbio = kzalloc_flex(*rbio, stripe_pages, num_pages, GFP_NOFS);
 	if (!rbio)
 		return ERR_PTR(-ENOMEM);
-	rbio->stripe_pages = kzalloc_objs(struct page *, num_pages, GFP_NOFS);
+
+	rbio->nr_pages = num_pages;
 	rbio->bio_paddrs = kzalloc_objs(phys_addr_t,
 					num_sectors * sector_nsteps, GFP_NOFS);
 	rbio->stripe_paddrs = kzalloc_objs(phys_addr_t,
@@ -1083,7 +1083,7 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
 	rbio->error_bitmap = bitmap_zalloc(num_sectors, GFP_NOFS);
 	rbio->stripe_uptodate_bitmap = bitmap_zalloc(num_sectors, GFP_NOFS);
 
-	if (!rbio->stripe_pages || !rbio->bio_paddrs || !rbio->stripe_paddrs ||
+	if (!rbio->bio_paddrs || !rbio->stripe_paddrs ||
 	    !rbio->finish_pointers || !rbio->error_bitmap || !rbio->stripe_uptodate_bitmap) {
 		free_raid_bio_pointers(rbio);
 		kfree(rbio);
@@ -1102,7 +1102,6 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
 	INIT_LIST_HEAD(&rbio->hash_list);
 	btrfs_get_bioc(bioc);
 	rbio->bioc = bioc;
-	rbio->nr_pages = num_pages;
 	rbio->nr_sectors = num_sectors;
 	rbio->real_stripes = real_stripes;
 	rbio->stripe_npages = stripe_npages;
diff --git a/fs/btrfs/raid56.h b/fs/btrfs/raid56.h
index 1f463ecf7e41..dbe6b7a2e194 100644
--- a/fs/btrfs/raid56.h
+++ b/fs/btrfs/raid56.h
@@ -194,12 +194,6 @@ struct btrfs_raid_bio {
 	 * allocated.
 	 */
 
-	/*
-	 * Pointers to pages that we allocated for reading/writing stripes
-	 * directly from the disk (including P/Q).
-	 */
-	struct page **stripe_pages;
-
 	/* Pointers to the sectors in the bio_list, for faster lookup */
 	phys_addr_t *bio_paddrs;
 
@@ -230,6 +224,12 @@ struct btrfs_raid_bio {
 	 * Should only cover data sectors (excluding P/Q sectors).
 	 */
 	unsigned long *csum_bitmap;
+
+	/*
+	 * Pointers to pages that we allocated for reading/writing stripes
+	 * directly from the disk (including P/Q).
+	 */
+	struct page *stripe_pages[] __counted_by(nr_pages);
 };
 
 /*
-- 
2.53.0

Re: [PATCH] btrfs: raid56: use kzalloc_flex for rbio

Posted by Qu Wenruo 1 month ago


在 2026/3/9 07:23, Rosen Penev 写道:
> Simplifies allocation slightly. Now stripe_pages does not need to be
> freed separately.
> 
> Added __counted_by for extra runtime analysis.

If that's the only variable sized member this will be a good change.

Unfortunately there are too many pointers for variable sized members, 
all around the old @stripe_pages pointer.

Treating @stripe_pages differently from other doesn't seem good to 
expandability nor readability.

Thanks,
Qu

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   fs/btrfs/raid56.c |  9 ++++-----
>   fs/btrfs/raid56.h | 12 ++++++------
>   2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index baebd9f733e9..d74ae380265e 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -151,7 +151,6 @@ static void free_raid_bio_pointers(struct btrfs_raid_bio *rbio)
>   {
>   	bitmap_free(rbio->error_bitmap);
>   	bitmap_free(rbio->stripe_uptodate_bitmap);
> -	kfree(rbio->stripe_pages);
>   	kfree(rbio->bio_paddrs);
>   	kfree(rbio->stripe_paddrs);
>   	kfree(rbio->finish_pointers);
> @@ -1070,10 +1069,11 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
>   	ASSERT(real_stripes >= 2);
>   	ASSERT(real_stripes <= U8_MAX);
>   
> -	rbio = kzalloc_obj(*rbio, GFP_NOFS);
> +	rbio = kzalloc_flex(*rbio, stripe_pages, num_pages, GFP_NOFS);
>   	if (!rbio)
>   		return ERR_PTR(-ENOMEM);
> -	rbio->stripe_pages = kzalloc_objs(struct page *, num_pages, GFP_NOFS);
> +
> +	rbio->nr_pages = num_pages;
>   	rbio->bio_paddrs = kzalloc_objs(phys_addr_t,
>   					num_sectors * sector_nsteps, GFP_NOFS);
>   	rbio->stripe_paddrs = kzalloc_objs(phys_addr_t,
> @@ -1083,7 +1083,7 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
>   	rbio->error_bitmap = bitmap_zalloc(num_sectors, GFP_NOFS);
>   	rbio->stripe_uptodate_bitmap = bitmap_zalloc(num_sectors, GFP_NOFS);
>   
> -	if (!rbio->stripe_pages || !rbio->bio_paddrs || !rbio->stripe_paddrs ||
> +	if (!rbio->bio_paddrs || !rbio->stripe_paddrs ||
>   	    !rbio->finish_pointers || !rbio->error_bitmap || !rbio->stripe_uptodate_bitmap) {
>   		free_raid_bio_pointers(rbio);
>   		kfree(rbio);
> @@ -1102,7 +1102,6 @@ static struct btrfs_raid_bio *alloc_rbio(struct btrfs_fs_info *fs_info,
>   	INIT_LIST_HEAD(&rbio->hash_list);
>   	btrfs_get_bioc(bioc);
>   	rbio->bioc = bioc;
> -	rbio->nr_pages = num_pages;
>   	rbio->nr_sectors = num_sectors;
>   	rbio->real_stripes = real_stripes;
>   	rbio->stripe_npages = stripe_npages;
> diff --git a/fs/btrfs/raid56.h b/fs/btrfs/raid56.h
> index 1f463ecf7e41..dbe6b7a2e194 100644
> --- a/fs/btrfs/raid56.h
> +++ b/fs/btrfs/raid56.h
> @@ -194,12 +194,6 @@ struct btrfs_raid_bio {
>   	 * allocated.
>   	 */
>   
> -	/*
> -	 * Pointers to pages that we allocated for reading/writing stripes
> -	 * directly from the disk (including P/Q).
> -	 */
> -	struct page **stripe_pages;
> -
>   	/* Pointers to the sectors in the bio_list, for faster lookup */
>   	phys_addr_t *bio_paddrs;
>   
> @@ -230,6 +224,12 @@ struct btrfs_raid_bio {
>   	 * Should only cover data sectors (excluding P/Q sectors).
>   	 */
>   	unsigned long *csum_bitmap;
> +
> +	/*
> +	 * Pointers to pages that we allocated for reading/writing stripes
> +	 * directly from the disk (including P/Q).
> +	 */
> +	struct page *stripe_pages[] __counted_by(nr_pages);
>   };
>   
>   /*