All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xiao Yang <yangx.jy@cn.fujitsu.com>
To: Ira Weiny <ira.weiny@intel.com>
Cc: Xiao Yang <ice_yangxiao@163.com>, <fstests@vger.kernel.org>,
	<darrick.wong@oracle.com>, <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH v2 6/6] xfs/260: Move xfs/260 to generic
Date: Tue, 23 Jun 2020 16:51:31 +0800	[thread overview]
Message-ID: <5EF1C293.4010104@cn.fujitsu.com> (raw)
In-Reply-To: <20200619151511.GR4160762@iweiny-DESK2.sc.intel.com>

On 2020/6/19 23:15, Ira Weiny wrote:
> On Thu, Jun 18, 2020 at 11:35:56PM +0800, Xiao Yang wrote:
>> On 6/18/20 5:48 AM, Ira Weiny wrote:
>>> On Wed, Jun 17, 2020 at 05:32:04PM +0800, Xiao Yang wrote:
>>>> From: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>>
>>>> Both ext4 and xfs support per-inode DAX flag now so move it to generic.
>>>>
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>> Unfortunately this test needs to be modified to work with the final agreed upon
>>> method for switching DAX, patch below.  Feel free to squash it into this patch
>>> for v3.
>>>
>>> There are more checks I had queued up but I'm happy to wait for this series to
>>> be applied.
>>>
>>> Again, thanks for moving this along!
>>> Ira
>>>
>>>   From 97b67a2773627cfe91b5f33a418ab646bb5fd0a8 Mon Sep 17 00:00:00 2001
>>> From: Ira Weiny<ira.weiny@intel.com>
>>> Date: Wed, 17 Jun 2020 14:34:45 -0700
>>> Subject: [PATCH] generic/602: Modify to work with agreed inheritance of DAX flag
>>>
>>> Modifying the DAX flag on flies does not take effect immediately.  The
>>> easiest way to ensure the file state for this test is to use inheritance
>>> of the parent directory DAX state.
>> Hi Ira,
>>
>> Modifying the DAX flag on directories can take effect immediately, right?
> Yes and any file created under that directory will inherit the DAX flag
> immediately.
>
>>> Modify the test to use 2 directories which we can switch the DAX state on
>>> and create the test files in those directories after setting the desired
>>> state.
>> Good addition, and I will squash it into this v3 patch. :-)
>>
>> BTW:
>>
>> I wait someone(Darrick, Eryu or others) to review the code about PMD fault
>> testing in this patch set.
> Yes, thanks.  I don't feel familiar enough with that code to review it.
>
> Ira
>
>> Best Regards,
>>
>> Xiao Yang
>>
>>> Signed-off-by: Ira Weiny<ira.weiny@intel.com>
>>> ---
>>>    tests/generic/602 | 81 +++++++++++++++++++++++++++++------------------
>>>    1 file changed, 51 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/tests/generic/602 b/tests/generic/602
>>> index 9137c5b9385f..20bf2c6bd246 100755
>>> --- a/tests/generic/602
>>> +++ b/tests/generic/602
>>> @@ -36,68 +36,95 @@ _require_test_program "t_mmap_dio"
>>>    _require_scratch_dax_iflag
>>>    _require_xfs_io_command "falloc"
>>> -prep_files()
>>> +SRC_DIR=$SCRATCH_MNT/src
>>> +SRC_FILE=$SRC_DIR/tf_s
>>> +
>>> +DST_DIR=$SCRATCH_MNT/dst
>>> +DST_FILE=$DST_DIR/tf_d
>>> +
>>> +clean_files()
>>>    {
>>> -	rm -f $SCRATCH_MNT/tf_{s,d}
>>> +	rm -f $SRC_FILE
>>> +	rm -f $DST_FILE
>>> +	mkdir -p $SRC_DIR
>>> +	mkdir -p $DST_DIR
>>> +}
>>> +
>>> +prep_files()
>>> +{
>>> +	$XFS_IO_PROG -f -c "falloc 0 $tsize" \
>>> +		$SRC_FILE>>  $seqres.full 2>&1
>>>    	$XFS_IO_PROG -f -c "falloc 0 $tsize" \
>>> -		$SCRATCH_MNT/tf_{s,d}>>  $seqres.full 2>&1
>>> +		$DST_FILE>>  $seqres.full 2>&1
>>>    }
>>>    t_both_dax()
>>>    {
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr +x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr +x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
>>>    	# with O_DIRECT first
>>> -	$here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} $1 "dio both dax"
>>> +	$here/src/t_mmap_dio $SRC_FILE $DST_FILE $1 "dio both dax"
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr +x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr +x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_{s,d}
>>>    	# again with buffered IO
>>> -	$here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
>>>    		$1 "buffered both dax"
>>>    }
>>>    t_nondax_to_dax()
>>>    {
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr -x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr +x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
>>> -	$here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio $SRC_FILE $DST_FILE \
>>>    		$1 "dio nondax to dax"
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr -x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr +x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_s
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_d
>>> -	$here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
>>>    		$1 "buffered nondax to dax"
>>>    }
>>>    t_dax_to_nondax()
>>>    {
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr +x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr -x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
>>> -	$here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio $SRC_FILE $DST_FILE \
>>>    		$1 "dio dax to nondax"
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr +x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr -x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr +x" $SCRATCH_MNT/tf_s
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_d
>>> -	$here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
>>>    		$1 "buffered dax to nondax"
>>>    }
>>>    t_both_nondax()
>>>    {
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr -x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr -x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
>>> -	$here/src/t_mmap_dio $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio $SRC_FILE $DST_FILE \
>>>    		$1 "dio both nondax"
>>> +	clean_files
>>> +	$XFS_IO_PROG -c "chattr -x" $SRC_DIR
>>> +	$XFS_IO_PROG -c "chattr -x" $DST_DIR
>>>    	prep_files
>>> -	$XFS_IO_PROG -c "chattr -x" $SCRATCH_MNT/tf_{s,d}
>>> -	$here/src/t_mmap_dio -b $SCRATCH_MNT/tf_{s,d} \
>>> +	$here/src/t_mmap_dio -b $SRC_FILE $DST_FILE \
>>>    		$1 "buffered both nondax"
>>>    }
>>> @@ -124,17 +151,11 @@ do_tests()
>>>    # make xfs aligned for PMD fault testing
>>>    _scratch_mkfs_geom $(_get_hugepagesize) 1>>  $seqres.full 2>&1
>>> -# mount with dax option
>>> -_scratch_mount "-o dax"
>>> -
Hi Ira,

Why do you want to remove this combination(i.e. test per-inode DAX flag 
under mounting with dax option) ?
Is it because mounting with dax option ignore FS_XFLAG_DAX flag?
I think it is a reasonable combination. :-)

>>>    tsize=$((128 * 1024 * 1024))
>>> -do_tests
>>> -_scratch_unmount
>>> -
>>>    # mount again without dax option
>>>    export MOUNT_OPTIONS=""
>>> -_scratch_mount
>>> +_scratch_mount "-o dax=inode"
>>>    do_tests
>>>    # success, all done

Could we keep _scratch_mount without dax so that this test can run on 
both old and new kernel?
See the following reasons:
1) FS_XFLAG_DAX was introduced by commit 58f88ca("xfs: introduce 
per-inode DAX enablement") since 2017.
2) _scratch_mount with dax=inode is equal to _scratch_mount without dax.

Best Regards,
Xiao Yang
>
> .
>




  reply	other threads:[~2020-06-23  8:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17  9:31 [PATCH v2 0/6] Make fstests support new behavior of DAX Xiao Yang
2020-06-17  9:31 ` [PATCH v2 1/6] common/rc: Introduce new helpers for DAX mount options and FS_XFLAG_DAX Xiao Yang
2020-06-17 20:58   ` Ira Weiny
2020-06-17  9:32 ` [PATCH v2 2/6] fstests: Use _require_scratch_dax_mountopt() and _require_scratch_dax_iflag() Xiao Yang
2020-06-17 20:58   ` Ira Weiny
2020-06-17  9:32 ` [PATCH v2 3/6] common/rc: Remove unused _require_scratch_dax() Xiao Yang
2020-06-17 20:59   ` Ira Weiny
2020-06-17  9:32 ` [PATCH v2 4/6] generic/223: Don't clear all mkfs options for _scratch_mkfs_geom() roughly Xiao Yang
2020-06-17  9:32 ` [PATCH v2 5/6] generic/413, xfs/260: Improve format operation for PMD fault testing Xiao Yang
2020-06-23  6:00   ` Xiao Yang
2020-06-23 15:28   ` Darrick J. Wong
2020-06-17  9:32 ` [PATCH v2 6/6] xfs/260: Move xfs/260 to generic Xiao Yang
2020-06-17 21:48   ` Ira Weiny
2020-06-18 15:35     ` Xiao Yang
2020-06-19 15:15       ` Ira Weiny
2020-06-23  8:51         ` Xiao Yang [this message]
2020-06-23 18:08           ` Ira Weiny
2020-06-30  7:08             ` Xiao Yang
2020-06-30 15:50               ` Ira Weiny
2020-07-01  1:07                 ` Xiao Yang
2020-06-18  0:21 ` [PATCH v2 0/6] Make fstests support new behavior of DAX Ira Weiny
2020-06-18 15:04   ` Xiao Yang

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=5EF1C293.4010104@cn.fujitsu.com \
    --to=yangx.jy@cn.fujitsu.com \
    --cc=darrick.wong@oracle.com \
    --cc=fstests@vger.kernel.org \
    --cc=ice_yangxiao@163.com \
    --cc=ira.weiny@intel.com \
    --cc=ross.zwisler@linux.intel.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.