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

[Qu Wenruo <wqu@suse.com>]

在 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);
>   };
>   
>   /*