Re: [PATCH] ext4: fix LOGFLUSH shutdown ordering to allow ordered-mode data writeback

[Baokun Li]

On 2026/4/24 18:42, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> In EXT4_GOING_FLAGS_LOGFLUSH mode, the EXT4_FLAGS_SHUTDOWN flag was set
> before calling ext4_force_commit().  This caused ordered-mode data
> writeback (triggered by journal commit) to fail with -EIO, since
> ext4_do_writepages() checks for the shutdown flag.  The journal would
> then be aborted prematurely before the commit could succeed.
>
> Fix this by calling ext4_force_commit() first, then setting the
> shutdown flag, so that pending data can be written back correctly.
>
> Note that moving ext4_force_commit() before setting the shutdown flag
> creates a small window in which new writes may occur and generate new
> journal transactions.  When the journal is subsequently aborted, the
> new transactions will not be able to write to disk.  This is intentional
> because LOGFLUSH's semantics are to flush pre-existing journal entries
> before shutdown, not to guarantee atomicity for writes that race with
> the ioctl.
>
> Fixes: 783d94854499 ("ext4: add EXT4_IOC_GOINGDOWN ioctl")
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Looks good to me.

Reviewed-by: Baokun Li <libaokun@xxxxxxxxxxxxxxxxx>

> ---
> This fix addresses my new generic/970 test, which fails during the file
> size verification after shutdown and remount.
>
>  https://lore.kernel.org/fstests/20260424092228.1396658-1-yi.zhang@xxxxxxxxxxxxxxx/
>
>  fs/ext4/ioctl.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 1d0c3d4bdf47..110e3fb194ec 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -830,11 +830,17 @@ int ext4_force_shutdown(struct super_block *sb, u32 flags)
>  		bdev_thaw(sb->s_bdev);
>  		break;
>  	case EXT4_GOING_FLAGS_LOGFLUSH:
> +		/*
> +		 * Call ext4_force_commit() before setting EXT4_FLAGS_SHUTDOWN.
> +		 * This is because in data=ordered mode, journal commit
> +		 * triggers data writeback which fails if shutdown is already
> +		 * set, causing the journal to be aborted prematurely before
> +		 * the commit succeeds.
> +		 */
> +		(void) ext4_force_commit(sb);
>  		set_bit(EXT4_FLAGS_SHUTDOWN, &sbi->s_ext4_flags);
> -		if (sbi->s_journal && !is_journal_aborted(sbi->s_journal)) {
> -			(void) ext4_force_commit(sb);
> +		if (sbi->s_journal && !is_journal_aborted(sbi->s_journal))
>  			jbd2_journal_abort(sbi->s_journal, -ESHUTDOWN);
> -		}
>  		break;
>  	case EXT4_GOING_FLAGS_NOLOGFLUSH:
>  		set_bit(EXT4_FLAGS_SHUTDOWN, &sbi->s_ext4_flags);