Re: [PATCH 1/2] xfs/438: retry failed metadata writes again

[Brian Foster <bfoster@redhat.com>]

On Thu, Nov 12, 2026 at 10:37:17AM +0900, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> xfs/438 checks that a dquot buffer write that keeps failing does not
> hang the unmount (kernel commit 373b0589dc8d). Commit eb2ad950ea90
> moved the test to _scratch_mount. Since commit 1a49022fab9b,
> _scratch_mount sets the metadata EIO retries to 0. XFS then shuts
> down on the second failure, before the AIL can retry the write. The
> test passes even with the bug.
> 
> Turn the retries back on after the mount. Fail the test if XFS has
> shut down before the unmount.
> 
> Fixes: eb2ad950ea90 ("dmflakey: override SCRATCH_DEV in _init_flakey")
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> Tested on kernels with and without XFS_DEBUG. Patch 1 gives the same
> results on both. With an internal log, v7.4-rc2 passes 3 of 3. A TEST
> ONLY kernel that skips the resubmission of failed AIL items hangs at
> unmount (1 of 1). Without patch 2, the new check fails in three cases:
> 
> - with the dm patch and a write cache (3 of 3)
> - with an external log (1 of 1)
> - with an rt device that has a write cache (from the code, not run)
> 
> In each, XFS shuts down on a log error before the unmount. Patch 2
> passes in the first two cases.
> ---
>  tests/xfs/438 | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/xfs/438 b/tests/xfs/438
> index 4cd0b26..e4850ff 100755
> --- a/tests/xfs/438
> +++ b/tests/xfs/438
...
> @@ -117,6 +120,12 @@ sysctl -w fs.xfs.xfssyncd_centisecs=100 >> $seqres.full 2>&1
>  _qmount_option "usrquota"
>  _scratch_mount
>  
> +# _scratch_mount turns off the retries of failed metadata writes.
> +# Go back to the kernel defaults, so that the failed dquot buffer
> +# write is retried from the AIL until unmount. fail_at_unmount
> +# stays on, so the unmount ends the retries on a fixed kernel.
> +_reset_xfs_sysfs_error_handling $SCRATCH_DEV >> $seqres.full
> +
>  # We need to set the quota limitation twice, and inject the write error
>  # after the second setting. If we try to inject the write error after
>  # the first setting, the initialization of the dquota buffer will get
> @@ -145,6 +154,10 @@ _scratch_sync
>  # the completion of the retried write of dquota buffer
>  sleep 2
>  
> +# The dquot buffer write must still be failing and retried here. A shut
> +# down filesystem means that the test missed the hang it checks.

Nit on the comment.. something like this is a little more clear to me:

# Run a stat to confirm we haven't shutdown prematurely. We expect the
# fs to be running metadata write retries. A shutdown fs at this point
# means that we haven't properly tested for the hang.

But this LGTM regardless. Maybe Zorro can fix that up if you agree:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +stat $SCRATCH_MNT > /dev/null
> +
>  _scratch_unmount
>  
>  _cleanup_flakey
> 
> -- 
> 2.43.0
> 
>