Re: [PATCH v3] xfs/12[4-6]: Add getfattr operation after xattr corrupted

[Eryu Guan <guan@eryu.me>]

On Mon, Jan 17, 2022 at 04:22:16PM +0800, Yang Xu wrote:
> Add getfattr operation in these three cases, we can also use xfs/126(corrupt a leaf
> xattr's data extent) to reproduce a deadlock. This deadlock is introduced by kernel
> commit 07120f1abdff ("xfs: Add xfs_has_attr and subroutines") and fixed by kernel

So this was introduced since 5.9 kernel, and

> commit a1de97fe296c ("xfs: Fix the free logic of state in xfs_attr_node_hasname").

fixed in 5.16 kernel.

And adding this regression test in xfs/126, which passed in the past.
probably would cause all the affected kernels in between start to hang,
and trigger a false positive regression alert.

So IMO it'd be better to make a separated & targeted regression test for
this case, as Darrick suggested.

Thanks,
Eryu

> 
> Also making getfattr operation after xattr corrupted is a common part, so we can
> test whether the similar deadlock occurs in the future in xfs/124(corrupt a block xattr)
> and xfs/125(corrupt a leaf xattr's index extent).
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  tests/xfs/124 | 2 +-
>  tests/xfs/125 | 2 +-
>  tests/xfs/126 | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/xfs/124 b/tests/xfs/124
> index 39307218..c3cccfd5 100755
> --- a/tests/xfs/124
> +++ b/tests/xfs/124
> @@ -64,7 +64,7 @@ _scratch_xfs_db -x -c "inode ${inode}" -c 'ablock 0' -c "stack" -c "blocktrash -
>  
>  echo "+ mount image && modify xattr"
>  if _try_scratch_mount >> $seqres.full 2>&1; then
> -
> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2> /dev/null && _fail "got corrupt xattr"
>  	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2> /dev/null && _fail "modified corrupt xattr"
>  	umount "${SCRATCH_MNT}"
>  fi
> diff --git a/tests/xfs/125 b/tests/xfs/125
> index fb5f5695..a7eb51fb 100755
> --- a/tests/xfs/125
> +++ b/tests/xfs/125
> @@ -64,7 +64,7 @@ _scratch_xfs_db -x -c "inode ${inode}" -c 'ablock 0' -c "stack" -c "blocktrash -
>  
>  echo "+ mount image && modify xattr"
>  if _try_scratch_mount >> $seqres.full 2>&1; then
> -
> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2> /dev/null && _fail "got corrupt xattr"
>  	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2> /dev/null && _fail "modified corrupt xattr"
>  	umount "${SCRATCH_MNT}"
>  fi
> diff --git a/tests/xfs/126 b/tests/xfs/126
> index c3a74b1c..519d377e 100755
> --- a/tests/xfs/126
> +++ b/tests/xfs/126
> @@ -69,7 +69,7 @@ done
>  
>  echo "+ mount image && modify xattr"
>  if _try_scratch_mount >> $seqres.full 2>&1; then
> -
> +	getfattr "${SCRATCH_MNT}/attrfile" -n "user.x00000000" 2> /dev/null && _fail "got corrupt xattr"
>  	setfattr -x "user.x00000000" "${SCRATCH_MNT}/attrfile" 2> /dev/null && _fail "modified corrupt xattr"
>  	umount "${SCRATCH_MNT}"
>  fi
> -- 
> 2.23.0