Re: [PATCH] btrfs: compression: allocate buckets with workspace

[Qu Wenruo <wqu@suse.com>]

在 2026/6/29 09:55, Rosen Penev 写道:
> Convert 3 allocations into one. Simplifies code slightly.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   fs/btrfs/compression.c | 18 +++++-------------
>   1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index ffb6b52863a7..da6749ff5924 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -650,11 +650,11 @@ struct heuristic_ws {
>   	/* Partial copy of input data */
>   	u8 *sample;
>   	u32 sample_size;
> -	/* Buckets store counters for each byte value */
> -	struct bucket_item *bucket;
>   	/* Sorting buffer */
>   	struct bucket_item *bucket_b;
>   	struct list_head list;
> +	/* Buckets store counters for each byte value */
> +	struct bucket_item bucket[];
>   };
>   
>   static void free_heuristic_ws(struct list_head *ws)
> @@ -664,8 +664,6 @@ static void free_heuristic_ws(struct list_head *ws)
>   	workspace = list_entry(ws, struct heuristic_ws, list);
>   
>   	kvfree(workspace->sample);
> -	kfree(workspace->bucket);
> -	kfree(workspace->bucket_b);
>   	kfree(workspace);
>   }
>   
> @@ -673,22 +671,16 @@ static struct list_head *alloc_heuristic_ws(struct btrfs_fs_info *fs_info)
>   {
>   	struct heuristic_ws *ws;
>   
> -	ws = kzalloc_obj(*ws);
> +	ws = kzalloc(struct_size(ws, bucket, BUCKET_SIZE * 2), GFP_KERNEL);
>   	if (!ws)
>   		return ERR_PTR(-ENOMEM);
>   
> +	ws->bucket_b = ws->bucket + BUCKET_SIZE;
> +

This barely improves any readability.

We have two different bucket pointers, one is a variable sized member, 
one (bucket_b) is a pointer.

Not a fan of this at all.
>   	ws->sample = kvmalloc(MAX_SAMPLE_SIZE, GFP_KERNEL);
>   	if (!ws->sample)
>   		goto fail;
>   
> -	ws->bucket = kzalloc_objs(*ws->bucket, BUCKET_SIZE);
> -	if (!ws->bucket)
> -		goto fail;
> -
> -	ws->bucket_b = kzalloc_objs(*ws->bucket_b, BUCKET_SIZE);
> -	if (!ws->bucket_b)
> -		goto fail;
> -
>   	INIT_LIST_HEAD(&ws->list);
>   	return &ws->list;
>   fail: