All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xiao Yang <yangx.jy@cn.fujitsu.com>
To: XiaoLi Feng <xifeng@redhat.com>
Cc: <fstests@vger.kernel.org>, <darrick.wong@oracle.com>
Subject: Re: [PATCH v2] generic/608: verify S_DAX is active immediately for DONTCACHE
Date: Thu, 18 Feb 2021 14:08:56 +0800	[thread overview]
Message-ID: <602E0478.401@cn.fujitsu.com> (raw)
In-Reply-To: <20210210075815.13958-1-xifeng@redhat.com>

On 2021/2/10 15:58, XiaoLi Feng wrote:
> From: Xiaoli Feng <xifeng@redhat.com>
>
> After upstream linux patches(77573fa310d, 88149082bb8), if
> FS_XFLAG_DAX is set, DONTCACHE is also set. So S_DAX on the file
> should be active immediately once FS_XFLAG_DAX is set.
> ---
>  tests/generic/608 | 38 +++++++++++---------------------------
>  1 file changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/tests/generic/608 b/tests/generic/608
> index dd89d91c..92f368e0 100755
> --- a/tests/generic/608
> +++ b/tests/generic/608
> @@ -6,7 +6,7 @@
>  # Change FS_XFLAG_DAX on an existing file and check if S_DAX on
>  # the file can take effect immediately by the following steps:
>  # 1) Stop all applications which are using the file.
> -# 2) Do drop_caches or umount & mount cycle.
> +# 2) DONTCACHE is set.
Hi XiaoLi,

It is better to add the related commits here.

>  
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> @@ -36,7 +36,7 @@ _require_dax_iflag
>  _require_xfs_io_command "lsattr" "-v"
>  _require_xfs_io_command "statx" "-r"
>  
> -test_drop_caches()
> +test_caches()
>  {
>  	local t_file=$SCRATCH_MNT/testfile
>  
> @@ -51,56 +51,40 @@ test_drop_caches()
>  	_check_xflag $t_file 1
>  	_check_s_dax $t_file 0
>  
> -	# One application is using test file and check if S_DAX on
> -	# the file doesn't take effect immediately by drop_caches
> -	echo 2 > /proc/sys/vm/drop_caches
> -	_check_s_dax $t_file 0

I want to add a similar comment in front of "_check_s_dax $t_file 0":
-------------------------------------
50 $XFS_IO_PROG -c 'chattr +x' $t_file
51 _check_xflag $t_file 1
52 # One application is using test file and check if S_DAX on
53 # the file doesn't take effect immediately
54 _check_s_dax $t_file 0
-------------------------------------

> -
>  	exec 3<&-
>  
>  	# No application is using test file and check if S_DAX on
>  	# the file takes effect immediately by drop_caches

Please remove the 'by drop_caches'

> -	echo 2 > /proc/sys/vm/drop_caches
>  	_check_s_dax $t_file 1
>  }
>  
> -test_cycle_mount()
> +test_nocache()
>  {
> -	local option=$1
> -	local t_dir=$SCRATCH_MNT/testdir
> -	local t_file=$t_dir/testfile
> +	local t_file=$SCRATCH_MNT/testnocache
>  
> -	mkdir -p $t_dir
> -	$XFS_IO_PROG -c 'chattr +x' $t_dir
>  	rm -f $t_file
>  	touch $t_file
> +	_check_xflag $t_file 0
> +	_check_s_dax $t_file 0

Inheriting xflag and S_DAX from the parent directory is a good variety
so I want to keep the $t_dir which has enabled xflag.

Best Regards,
Xiao Yang

> +
> +	$XFS_IO_PROG -c 'chattr +x' $t_file
>  	_check_xflag $t_file 1
>  	_check_s_dax $t_file 1
>  
> -	exec 3< $t_file
> -
>  	$XFS_IO_PROG -c 'chattr -x' $t_file
>  	_check_xflag $t_file 0
> -	_check_s_dax $t_file 1
> -
> -	exec 3<&-
> -
> -	# No application is using test file and check if S_DAX on
> -	# the file takes effect immediately by umount & mount
> -	_scratch_cycle_mount "$option"
>  	_check_s_dax $t_file 0
>  }
>  
>  do_tests()
>  {
>  	local mount_option=$1
> -	local cycle_mount_option=$2
>  
>  	_scratch_mount "$mount_option"
>  
> -	test_drop_caches
> +	test_caches
>  
> -	test_cycle_mount "$cycle_mount_option"
> +	test_nocache
>  
>  	_scratch_unmount
>  }
> @@ -108,7 +92,7 @@ do_tests()
>  _scratch_mkfs >> $seqres.full 2>&1
>  
>  # Mount with dax option
> -do_tests "-o dax=inode" "dax=inode"
> +do_tests "-o dax=inode"
>  
>  # Mount without dax option
>  export MOUNT_OPTIONS=""




      reply	other threads:[~2021-02-18  6:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10  7:58 [PATCH v2] generic/608: verify S_DAX is active immediately for DONTCACHE XiaoLi Feng
2021-02-18  6:08 ` Xiao Yang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=602E0478.401@cn.fujitsu.com \
    --to=yangx.jy@cn.fujitsu.com \
    --cc=darrick.wong@oracle.com \
    --cc=fstests@vger.kernel.org \
    --cc=xifeng@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.