Re: [PATCH] ext4: reject delalloc to nodelalloc before applying remount options

[Ojaswin Mujoo]

On Fri, Aug 14, 2026 at 11:48:55AM +0800, guzebing wrote:
> Commit 97f5ec3b166d ("ext4: prevent delalloc to nodelalloc on
> remount") rejects switching a mounted filesystem from delalloc to
> nodelalloc.  However, it performs the check after ext4_apply_options()
> has already cleared EXT4_MOUNT_DELALLOC in the live superblock.  The
> failure path eventually restores the bit, but leaves a window where
> other CPUs can observe nodelalloc.
> 
> test_opt() reads s_mount_opt directly without a lock shared with
> remount.  If a concurrent truncate's __es_remove_extent() hits this
> window and observes nodelalloc, it sets count_reserved to false.
> Delayed extent status entries are then removed without calculating
> their cluster reservations, leaving reserved at zero.
> ext4_es_remove_extent() therefore calls ext4_da_release_space() with
> zero, leaving i_reserved_data_blocks and s_dirtyclusters_counter
> elevated.  When the inode is later evicted after unlink,
> ext4_destroy_inode() reports:
> 
>   i_reserved_data_blocks (...) not cleared!
> 
> CPU 0                             CPU 1
>                                   ksys_truncate()
>                                     ...
>                                     ext4_es_remove_extent()
> ext4_reconfigure()
>   ext4_check_opt_consistency()
>   __ext4_remount()
>     ext4_apply_options()
>       clear EXT4_MOUNT_DELALLOC
>                                       __es_remove_extent()
>                                         test_opt() sees !DELALLOC
>                                         count_reserved = false
>     reject delalloc -> nodelalloc
>     restore EXT4_MOUNT_DELALLOC
>                                         remove delayed ES
>                                     ext4_da_release_space(0)
> 

Hi guzebing,

This looks like a valid issue. I think the unrelated issues sashiko has
pointed is also valid and we can probably move back more of these checks
from __ext4_remount to ext4_check_opt_consistency, but that can be done
in separate patches.

Feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

> Follow the pre-apply validation approach used by
> ext4_check_quota_consistency() and reject the transition in
> ext4_check_opt_consistency(), before ext4_apply_options() changes live
> state.  Use mask_s_mount_opt to determine whether delalloc/nodelalloc
> was specified and ctx_test_mount_opt() to check the final parsed value.
> 
> Fixes: 97f5ec3b166d ("ext4: prevent delalloc to nodelalloc on remount")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: guzebing <guzebing1612@xxxxxxxxx>
> ---
> 
> This issue was first observed in a production environment.  It can now
> be reproduced with a shell script.
> 
>   1. Create a 768 MiB ext4 filesystem and mount it with delalloc.
>   2. Start 16 workers.  Each worker repeatedly runs:
> 
>        xfs_io -f -c 'pwrite -q 0 32m' file-N
>        truncate -s 262144 file-N
> 
>   3. In parallel, repeatedly run "mount -o remount,nodelalloc".  Every
>      remount is expected to fail.
>   4. Continuously sample the live mount options with findmnt and record
>      any transient nodelalloc state.
>   5. After 300 seconds, stop the workers and unmount the filesystem,
>      then inspect dmesg for "i_reserved_data_blocks (...) not cleared!".
> 
> On an affected kernel, 7 transient nodelalloc samples and 9 reservation
> warnings were observed during 101 rejected remounts.
> 
> With this fix, no transient nodelalloc state or reservation warning was
> observed.

Can you please add an xfstests for this. The test can stress this path and
then use the _check_dmesg_for helper to detect if there were any
i_reserved_data_blocks related warnings.

Regards,
ojaswin

> 
>  fs/ext4/super.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 245f67d10ded3..9e1ea94664775 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2816,6 +2816,14 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
>  	}
>  
>  	if (is_remount) {
> +		if (test_opt(sb, DELALLOC) &&
> +		    (ctx->mask_s_mount_opt & EXT4_MOUNT_DELALLOC) &&
> +		    !ctx_test_mount_opt(ctx, EXT4_MOUNT_DELALLOC)) {
> +			ext4_msg(sb, KERN_ERR,
> +				 "can't disable delalloc during remount");
> +			return -EINVAL;
> +		}
> +
>  		if (!sbi->s_journal &&
>  		    ctx_test_mount_opt(ctx, EXT4_MOUNT_DATA_ERR_ABORT)) {
>  			ext4_msg(NULL, KERN_WARNING,
> @@ -6660,13 +6668,6 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb)
>  		goto restore_opts;
>  	}
>  
> -	if ((old_opts.s_mount_opt & EXT4_MOUNT_DELALLOC) &&
> -	    !test_opt(sb, DELALLOC)) {
> -		ext4_msg(sb, KERN_ERR, "can't disable delalloc during remount");
> -		err = -EINVAL;
> -		goto restore_opts;
> -	}
> -
>  	sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
>  		(test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);
>