Re: [PATCH v2] generic/623: fix test for runing on overlayfs

[Dave Chinner <david@fromorbit.com>]

On Wed, Jun 01, 2022 at 03:34:06PM +0300, Amir Goldstein wrote:
> For this test to run on overlayfs we open a different file to perform
> shutdown while keeping the writeback target file open.
> 
> xfs_io -c fsync perform fsync also on the writeback target file, which
> is needed for triggering the write fault.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Zorro,
> 
> Following your comment on v1, this version does not change the
> behavior of the test when running on non-overlayfs.
> 
> I tested that this test passes for both xfs and overlayfs+xfs on v5.18
> and tested that both configs fail with the same warning on v5.10.109.
> 
> Thanks,
> Amir.
> 
>  tests/generic/623 | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/generic/623 b/tests/generic/623
> index ea016d91..5971717c 100755
> --- a/tests/generic/623
> +++ b/tests/generic/623
> @@ -24,10 +24,22 @@ _scratch_mount
>  # XFS had a regression where it failed to check shutdown status in the fault
>  # path. This produced an iomap warning because writeback failure clears Uptodate
>  # status on the page.
> +
> +# For this test to run on overlayfs we open a different file to perform
> +# shutdown while keeping the writeback target file open.
> +# xfs_io -c fsync post-shutdown performs fsync also on the writeback target file,
> +# which is critical for trigerring the writeback failure.
> +shutdown_cmd=()
> +shutdown_handle="$(_scratch_shutdown_handle)"
> +if [ "$shutdown_handle" != "$SCRATCH_MNT" ];then
> +	shutdown_cmd+=("-c" "open $shutdown_handle")
> +fi
> +shutdown_cmd+=("-c" "shutdown")

IMO, this is unnecessary complexity. The original patch with the
"fsync acts on all open files" comment above explains the xfs_io
fsync quirk that enables the test to do what it is supposed to be
doing without any of the this conditional command construction.

The less special case handling we need to splice into the test code,
the better.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com