[PATCH v2] common/rc: not run swapfile test for DAX

[PATCH v2] common/rc: not run swapfile test for DAX

[XiaoLi Feng]

From: Xiaoli Feng <xifeng@redhat.com>

DAX doesn't support swapfile. Without this patch,
_require_scratch_swapfile always returns fail for fsdax. Now
change to notrun.

Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
---
 common/rc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 11ff7635..c3c40f30 100644
--- a/common/rc
+++ b/common/rc
@@ -2461,8 +2461,13 @@ _require_scratch_swapfile()
 	case "$FSTYP" in
 	ext2|ext3|ext4|xfs)
 		if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then
-			_scratch_unmount
-			_fail "swapon failed for $FSTYP"
+			if _check_s_dax "$SCRATCH_MNT/swap" 1 >/dev/null; then
+				_scratch_unmount
+				_notrun "swapfiles are not supported"
+			else
+				_scratch_unmount
+				_fail "swapon failed for $FSTYP"
+			fi
 		fi
 		;;
 	*)
@@ -3326,6 +3331,7 @@ _check_s_dax()
 {
 	local target=$1
 	local exp_s_dax=$2
+	local ret=0
 
 	local attributes=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
 
@@ -3347,9 +3353,12 @@ _check_s_dax()
 
 	if [ $exp_s_dax -eq 0 ]; then
 		(( attributes & 0x00200000 )) && echo "$target has unexpected S_DAX flag"
+		(( attributes & 0x00200000 )) && ret=1
 	else
 		(( attributes & 0x00200000 )) || echo "$target doesn't have expected S_DAX flag"
+		(( attributes & 0x00200000 )) || ret=2
 	fi
+	return $ret
 }
 
 _check_xflag()
-- 
2.18.1

Re: [PATCH v2] common/rc: not run swapfile test for DAX

[Darrick J. Wong]

On Sun, Apr 25, 2021 at 12:15:23AM +0800, XiaoLi Feng wrote:
> From: Xiaoli Feng <xifeng@redhat.com>
> 
> DAX doesn't support swapfile. Without this patch,
> _require_scratch_swapfile always returns fail for fsdax. Now
> change to notrun.
> 
> Signed-off-by: Xiaoli Feng <xifeng@redhat.com>

Sorry about my earlier mutterings, I forgot that I'm (still) so far
behind on sending fstests patches upstream that I hadn't actually gotten
to the one that fixes this.  Hence my earlier questions about "can't you
just use the existing S_DAX check?" make no sense.

> ---
>  common/rc | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 11ff7635..c3c40f30 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2461,8 +2461,13 @@ _require_scratch_swapfile()
>  	case "$FSTYP" in
>  	ext2|ext3|ext4|xfs)
>  		if ! swapon "$SCRATCH_MNT/swap" >/dev/null 2>&1; then
> -			_scratch_unmount
> -			_fail "swapon failed for $FSTYP"
> +			if _check_s_dax "$SCRATCH_MNT/swap" 1 >/dev/null; then
> +				_scratch_unmount
> +				_notrun "swapfiles are not supported"
> +			else
> +				_scratch_unmount
> +				_fail "swapon failed for $FSTYP"
> +			fi
>  		fi
>  		;;
>  	*)
> @@ -3326,6 +3331,7 @@ _check_s_dax()
>  {
>  	local target=$1
>  	local exp_s_dax=$2
> +	local ret=0
>  
>  	local attributes=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
>  
> @@ -3347,9 +3353,12 @@ _check_s_dax()
>  
>  	if [ $exp_s_dax -eq 0 ]; then
>  		(( attributes & 0x00200000 )) && echo "$target has unexpected S_DAX flag"
> +		(( attributes & 0x00200000 )) && ret=1
>  	else
>  		(( attributes & 0x00200000 )) || echo "$target doesn't have expected S_DAX flag"
> +		(( attributes & 0x00200000 )) || ret=2

Though I suppose if we want to pick nits here, it's kind of silly to
test this twice:

		if (( attributes & 0x00200000 )); then
			echo "$target doesn't have expected S_DAX_flag"
			return 2
		fi

OTOH, that's fairly minor so:

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  	fi
> +	return $ret
>  }
>  
>  _check_xflag()
> -- 
> 2.18.1
>