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

[Sun YangKai <sunk67188@gmail.com>]

On 2026/6/29 08:25, Rosen Penev wrote:
> 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);

It seems that size is fixed and known at compile time, and since we want 
to inline the buckets in the struct, I think we can have something like 
this to save one pointer:

struct heuristic_ws {
	...
	struct bucket_item bucket[BUCKET_SIZE];
	struct bucket_item bucket_b[BUCKET_SIZE];
}

However, each bucket array takes exactly 1024B memory. Currently we have 
a 48B allocation for the struct it self, 2 * 1024B allocation for bucket 
array, and 8192B allocation for sample. With these 2 arrays inlined, the 
struct will need 32 + 2048B allocation if my calculation is correct, 
which will goes into kmalloc-4k. Seems not good.

Thanks,
Sun YangKai

>   	if (!ws)
>   		return ERR_PTR(-ENOMEM);
>   
> +	ws->bucket_b = ws->bucket + BUCKET_SIZE;
> +
>   	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: