Re: [PATCH v4] ext4: simplify mballoc preallocation size rounding for small files

[Ojaswin Mujoo]

On Wed, Feb 25, 2026 at 01:02:31PM +0800, Weixie Cui wrote:
> From: Weixie Cui <cuiweixie@xxxxxxxxx>
> 
> The if-else ladder in ext4_mb_normalize_request() manually rounds up
> the preallocation size to the next power of two for files up to 1MB,
> enumerating each step from 16KB to 1MB individually. Replace this with
> a single roundup_pow_of_two() call clamped to a 16KB minimum, which
> is functionally equivalent but much more concise.
> 
> Also replace raw byte constants with SZ_1M and SZ_16K from
> <linux/sizes.h> for clarity, and remove the stale "XXX: should this
> table be tunable?" comment that has been there since the original
> mballoc code.
> 
> No functional change.
> 
> Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> Signed-off-by: Weixie Cui <cuiweixie@xxxxxxxxx>
> 
> ---
> v4:
>  - Drop unnecessary braces around single-line if/else as suggested
>    by Andreas Dilger
> 
> v3:
>  - Replace raw constants with SZ_1M/SZ_16K
>  - Remove stale XXX comment
> ---
>  fs/ext4/mballoc.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 20e9fdaf4301..1d6efba97835 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4561,22 +4561,16 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>  		(req <= (size) || max <= (chunk_size))
>  
>  	/* first, try to predict filesize */
> -	/* XXX: should this table be tunable? */
>  	start_off = 0;
> -	if (size <= 16 * 1024) {
> -		size = 16 * 1024;
> -	} else if (size <= 32 * 1024) {
> -		size = 32 * 1024;
> -	} else if (size <= 64 * 1024) {
> -		size = 64 * 1024;
> -	} else if (size <= 128 * 1024) {
> -		size = 128 * 1024;
> -	} else if (size <= 256 * 1024) {
> -		size = 256 * 1024;
> -	} else if (size <= 512 * 1024) {
> -		size = 512 * 1024;
> -	} else if (size <= 1024 * 1024) {
> -		size = 1024 * 1024;
> +	if (size <= SZ_1M) {
> +		/*
> +		 * For files up to 1MB, round up the preallocation size to
> +		 * the next power of two, with a minimum of 16KB.
> +		 */
> +		if (size <= (unsigned long)SZ_16K)
> +			size = SZ_16K;
> +		else
> +			size = roundup_pow_of_two(size);

Hi Weixie,

This if else ladder was long due a cleanup so thanks for taking this up.
I have one suggestion since you are already looking into this, do you
mind also cleaning up the NRL_CHECK_SIZE() logic because its pretty
messy

Essentially, the max contiguous blocks the buddy allocator can allocate
in one go is (bs * 2) no. of blocks so 4k bs can support 8192 blocks (ie
32MB) and 1024 bs can support 2048 blocks (ie 2MB) in one go. The
NRL_CHECK_SIZE() macro logic just makes sure that we dont set size as,
for example, 8MB for a 1kb bs ext4 which can only support upro 2MB
continuous allocations via buddy.

We should be able to refactor the above logic in much simpler way and
get rid of NRL_CHECK_SIZE.

Regards,
Ojaswin

>  	} else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, 2 * 1024)) {
>  		start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
>  						(21 - bsbits)) << 21;
> -- 
> 2.39.5 (Apple Git-154)
>