ext4: fix LOGFLUSH shutdown ordering to allow ordered-mode data writeback

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

Posted by Zhang Yi 1 month, 3 weeks ago

From: Zhang Yi <yi.zhang@huawei.com>

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@huawei.com>
---
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@huaweicloud.com/

 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);
-- 
2.52.0

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

Posted by Theodore Ts'o 2 weeks ago

On Fri, 24 Apr 2026 18:42:01 +0800, Zhang Yi wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] ext4: fix LOGFLUSH shutdown ordering to allow ordered-mode data writeback
      commit: d99748ef1695ce17eaf51c64b7a06952fa7cddab

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

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

Posted by Baokun Li 1 month, 3 weeks ago

On 2026/4/24 18:42, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> 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@huawei.com>

Looks good to me.

Reviewed-by: Baokun Li <libaokun@linux.alibaba.com>

> ---
> 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@huaweicloud.com/
>
>  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);

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

Posted by Jan Kara 1 month, 3 weeks ago

On Fri 24-04-26 18:42:01, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> 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@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> 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@huaweicloud.com/
> 
>  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);
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR