* [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size
@ 2022-10-23 6:48 yangx.jy
2022-10-23 6:48 ` [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z yangx.jy
2022-11-13 17:54 ` [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size Darrick J. Wong
0 siblings, 2 replies; 10+ messages in thread
From: yangx.jy @ 2022-10-23 6:48 UTC (permalink / raw)
To: djwong, zlang; +Cc: fstests, bfoster, Yasunori Gotou (Fujitsu), yangx.jy
It is unnecssary to always create a log-writes device based on
the size of the entire underlying device.
Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
---
common/dmlogwrites | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/common/dmlogwrites b/common/dmlogwrites
index 9fa1c977..d7b23cec 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -61,12 +61,20 @@ _require_log_writes_dax_mountopt()
_log_writes_init()
{
- blkdev=$1
+ local blkdev=$1
+ local range=$2
+ local BLK_DEV_SIZE
[ -z "$blkdev" ] && _fail \
"block dev must be specified for _log_writes_init"
- local BLK_DEV_SIZE=`blockdev --getsz $blkdev`
+ if [ -z "$range" ]; then
+ BLK_DEV_SIZE=`blockdev --getsz $blkdev`
+ else
+ local blksz=`blockdev --getss $blkdev`
+ BLK_DEV_SIZE=$((range / blksz))
+ fi
+
LOGWRITES_NAME=logwrites-test
LOGWRITES_DMDEV=/dev/mapper/$LOGWRITES_NAME
LOGWRITES_TABLE="0 $BLK_DEV_SIZE log-writes $blkdev $LOGWRITES_DEV"
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-23 6:48 [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size yangx.jy
@ 2022-10-23 6:48 ` yangx.jy
2022-10-24 4:09 ` Darrick J. Wong
2022-11-13 17:57 ` Darrick J. Wong
2022-11-13 17:54 ` [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size Darrick J. Wong
1 sibling, 2 replies; 10+ messages in thread
From: yangx.jy @ 2022-10-23 6:48 UTC (permalink / raw)
To: djwong, zlang; +Cc: fstests, bfoster, Yasunori Gotou (Fujitsu), yangx.jy
generic/470 was original designed to verify mmap(MAP_SYNC) which
is only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
introduced to fix the inconsistent filesystem issue[2] but it make
the test become not run because it doesn't support DAX. As Darrick
mentioned[3], discarding the entire mapped range of scartch device
can fix the issue as well, so I try to use blkdiscard -z instead.
[1]: https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=fc5870da485aec0f9196a0f2bed32f73f6b2c664
[2]: https://lore.kernel.org/fstests/20190227061529.GF16436@dastard/
[3]: https://lore.kernel.org/linux-xfs/Y1NRNtToQTjs0Dbd@magnolia/T/#me0e77cb0ecd80bf4b5109e4433cb4863ae6e6727
Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
---
tests/generic/470 | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/tests/generic/470 b/tests/generic/470
index f3407511..d2846a38 100755
--- a/tests/generic/470
+++ b/tests/generic/470
@@ -15,38 +15,29 @@ _cleanup()
{
cd /
_log_writes_cleanup
- _dmthin_cleanup
rm -f $tmp.*
}
# Import common functions.
. ./common/filter
-. ./common/dmthin
. ./common/dmlogwrites
# real QA test starts here
_supported_fs generic
-_require_scratch_nocheck
+_require_scratch
_require_no_logdev
_require_log_writes_dax_mountopt "dax"
-_require_dm_target thin-pool
_require_xfs_io_command "mmap" "-S"
_require_xfs_io_command "log_writes"
+_require_command "$BLKDISCARD_PROG" blkdiscard
-devsize=$((1024*1024*200 / 512)) # 200m phys/virt size
-csize=$((1024*64 / 512)) # 64k cluster size
-lowspace=$((1024*1024 / 512)) # 1m low space threshold
-
-# Use a thin device to provide deterministic discard behavior. Discards are used
-# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
-_dmthin_init $devsize $devsize $csize $lowspace
+RANGE=$((512 * 1024 * 1024)) # 512 MiB
+LEN=$((1024 * 1024)) # 1 MiB
-_log_writes_init $DMTHIN_VOL_DEV
+_log_writes_init $SCRATCH_DEV $RANGE
_log_writes_mkfs >> $seqres.full 2>&1
_log_writes_mount -o dax
-LEN=$((1024 * 1024)) # 1 MiB
-
$XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
-c "log_writes -d $LOGWRITES_NAME -m preunmap" \
-f $SCRATCH_MNT/test
@@ -54,14 +45,16 @@ $XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
# Unmount the scratch dir and tear down the log writes target
_log_writes_unmount
_log_writes_remove
-_dmthin_check_fs
+_check_scratch_fs
-# destroy previous filesystem so we can be sure our rebuild works
-_mkfs_dev $DMTHIN_VOL_DEV >> $seqres.full 2>&1
+# Discard the mapped range of scratch device and destroy previous filesystem
+# so we can be sure our rebuild works
+$BLKDISCARD_PROG -fzl $RANGE $SCRATCH_DEV >> $seqres.full 2>&1
+_scratch_mkfs >> $seqres.full 2>&1
# check pre-unmap state
-_log_writes_replay_log preunmap $DMTHIN_VOL_DEV
-_dmthin_mount
+_log_writes_replay_log preunmap $SCRATCH_DEV
+_scratch_mount
# We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-23 6:48 ` [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z yangx.jy
@ 2022-10-24 4:09 ` Darrick J. Wong
2022-10-24 7:15 ` Yang, Xiao/杨 晓
2022-11-13 17:57 ` Darrick J. Wong
1 sibling, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2022-10-24 4:09 UTC (permalink / raw)
To: yangx.jy; +Cc: zlang, fstests, bfoster, Yasunori Gotou (Fujitsu)
On Sun, Oct 23, 2022 at 06:48:13AM +0000, yangx.jy@fujitsu.com wrote:
> generic/470 was original designed to verify mmap(MAP_SYNC) which
> is only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
> introduced to fix the inconsistent filesystem issue[2] but it make
> the test become not run because it doesn't support DAX. As Darrick
> mentioned[3], discarding the entire mapped range of scartch device
> can fix the issue as well, so I try to use blkdiscard -z instead.
That might be ok for the *other* dm-logwrites tests, but isn't the
fundamental problem here (generic/470, specifically) that device mapper
cannot run on top of pmem?
--D
> [1]: https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=fc5870da485aec0f9196a0f2bed32f73f6b2c664
> [2]: https://lore.kernel.org/fstests/20190227061529.GF16436@dastard/
> [3]: https://lore.kernel.org/linux-xfs/Y1NRNtToQTjs0Dbd@magnolia/T/#me0e77cb0ecd80bf4b5109e4433cb4863ae6e6727
>
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
> tests/generic/470 | 31 ++++++++++++-------------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/tests/generic/470 b/tests/generic/470
> index f3407511..d2846a38 100755
> --- a/tests/generic/470
> +++ b/tests/generic/470
> @@ -15,38 +15,29 @@ _cleanup()
> {
> cd /
> _log_writes_cleanup
> - _dmthin_cleanup
> rm -f $tmp.*
> }
>
> # Import common functions.
> . ./common/filter
> -. ./common/dmthin
> . ./common/dmlogwrites
>
> # real QA test starts here
> _supported_fs generic
> -_require_scratch_nocheck
> +_require_scratch
> _require_no_logdev
> _require_log_writes_dax_mountopt "dax"
> -_require_dm_target thin-pool
> _require_xfs_io_command "mmap" "-S"
> _require_xfs_io_command "log_writes"
> +_require_command "$BLKDISCARD_PROG" blkdiscard
>
> -devsize=$((1024*1024*200 / 512)) # 200m phys/virt size
> -csize=$((1024*64 / 512)) # 64k cluster size
> -lowspace=$((1024*1024 / 512)) # 1m low space threshold
> -
> -# Use a thin device to provide deterministic discard behavior. Discards are used
> -# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
> -_dmthin_init $devsize $devsize $csize $lowspace
> +RANGE=$((512 * 1024 * 1024)) # 512 MiB
> +LEN=$((1024 * 1024)) # 1 MiB
>
> -_log_writes_init $DMTHIN_VOL_DEV
> +_log_writes_init $SCRATCH_DEV $RANGE
> _log_writes_mkfs >> $seqres.full 2>&1
> _log_writes_mount -o dax
>
> -LEN=$((1024 * 1024)) # 1 MiB
> -
> $XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> -c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> -f $SCRATCH_MNT/test
> @@ -54,14 +45,16 @@ $XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> # Unmount the scratch dir and tear down the log writes target
> _log_writes_unmount
> _log_writes_remove
> -_dmthin_check_fs
> +_check_scratch_fs
>
> -# destroy previous filesystem so we can be sure our rebuild works
> -_mkfs_dev $DMTHIN_VOL_DEV >> $seqres.full 2>&1
> +# Discard the mapped range of scratch device and destroy previous filesystem
> +# so we can be sure our rebuild works
> +$BLKDISCARD_PROG -fzl $RANGE $SCRATCH_DEV >> $seqres.full 2>&1
> +_scratch_mkfs >> $seqres.full 2>&1
>
> # check pre-unmap state
> -_log_writes_replay_log preunmap $DMTHIN_VOL_DEV
> -_dmthin_mount
> +_log_writes_replay_log preunmap $SCRATCH_DEV
> +_scratch_mount
>
> # We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-24 4:09 ` Darrick J. Wong
@ 2022-10-24 7:15 ` Yang, Xiao/杨 晓
2022-10-30 7:30 ` yangx.jy
0 siblings, 1 reply; 10+ messages in thread
From: Yang, Xiao/杨 晓 @ 2022-10-24 7:15 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: zlang, fstests, bfoster
On 2022/10/24 12:09, Darrick J. Wong wrote:
> On Sun, Oct 23, 2022 at 06:48:13AM +0000,yangx.jy@fujitsu.com wrote:
>> generic/470 was original designed to verify mmap(MAP_SYNC) which
>> is only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
>> introduced to fix the inconsistent filesystem issue[2] but it make
>> the test become not run because it doesn't support DAX. As Darrick
>> mentioned[3], discarding the entire mapped range of scartch device
>> can fix the issue as well, so I try to use blkdiscard -z instead.
> That might be ok for the*other* dm-logwrites tests, but isn't the
> fundamental problem here (generic/470, specifically) that device mapper
> cannot run on top of pmem?
Hi Darrick,
With the change，I didn't find any failure when running generic/470 in
loops.
--------------------------------------------------------------
[root@fedora35 xfstests-dev]# ./check generic/470
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 fedora35 6.1.0-rc1+ #37 SMP
PREEMPT_DYNAMIC Fri Oct 21 19:04:57 CST 2022
MKFS_OPTIONS -- -f /dev/pmem0
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/pmem0
/mnt/scratch
generic/470 6s
Ran: generic/470
Passed all 1 tests
--------------------------------------------------------------
Both dm-log-writes and PMEM support DAX so it's fine to verify
mmap(MAP_SYNC) with the dm-log-writes device on top of PMEM.
Did I miss something? Why do you think there is a fundamental problem here?
Best Regards,
Xiao Yang
>
> --D
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-24 7:15 ` Yang, Xiao/杨 晓
@ 2022-10-30 7:30 ` yangx.jy
2022-11-01 3:48 ` Darrick J. Wong
0 siblings, 1 reply; 10+ messages in thread
From: yangx.jy @ 2022-10-30 7:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: zlang, fstests, bfoster
Hi Darrick,
Ping, is there any feedback on the patch?
Best Regards,
Xiao Yang
-----Original Message-----
From: Yang, Xiao/杨 晓 <yangx.jy@fujitsu.com>
Sent: 2022年10月24日 15:16
To: Darrick J. Wong <djwong@kernel.org>
Cc: zlang@redhat.com; fstests@vger.kernel.org; bfoster@redhat.com
Subject: Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
On 2022/10/24 12:09, Darrick J. Wong wrote:
> On Sun, Oct 23, 2022 at 06:48:13AM +0000,yangx.jy@fujitsu.com wrote:
>> generic/470 was original designed to verify mmap(MAP_SYNC) which is
>> only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
>> introduced to fix the inconsistent filesystem issue[2] but it make
>> the test become not run because it doesn't support DAX. As Darrick
>> mentioned[3], discarding the entire mapped range of scartch device
>> can fix the issue as well, so I try to use blkdiscard -z instead.
> That might be ok for the*other* dm-logwrites tests, but isn't the
> fundamental problem here (generic/470, specifically) that device
> mapper cannot run on top of pmem?
Hi Darrick,
With the change，I didn't find any failure when running generic/470 in loops.
--------------------------------------------------------------
[root@fedora35 xfstests-dev]# ./check generic/470
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 fedora35 6.1.0-rc1+ #37 SMP
PREEMPT_DYNAMIC Fri Oct 21 19:04:57 CST 2022 MKFS_OPTIONS -- -f /dev/pmem0 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/pmem0 /mnt/scratch
generic/470 6s
Ran: generic/470
Passed all 1 tests
--------------------------------------------------------------
Both dm-log-writes and PMEM support DAX so it's fine to verify
mmap(MAP_SYNC) with the dm-log-writes device on top of PMEM.
Did I miss something? Why do you think there is a fundamental problem here?
Best Regards,
Xiao Yang
>
> --D
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-30 7:30 ` yangx.jy
@ 2022-11-01 3:48 ` Darrick J. Wong
2022-11-12 5:48 ` Yang, Xiao/杨 晓
0 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2022-11-01 3:48 UTC (permalink / raw)
To: yangx.jy; +Cc: zlang, fstests, bfoster
On Sun, Oct 30, 2022 at 07:30:31AM +0000, yangx.jy@fujitsu.com wrote:
> Hi Darrick,
>
> Ping, is there any feedback on the patch?
>
> Best Regards,
> Xiao Yang
>
> -----Original Message-----
> From: Yang, Xiao/杨 晓 <yangx.jy@fujitsu.com>
> Sent: 2022年10月24日 15:16
> To: Darrick J. Wong <djwong@kernel.org>
> Cc: zlang@redhat.com; fstests@vger.kernel.org; bfoster@redhat.com
> Subject: Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
>
> On 2022/10/24 12:09, Darrick J. Wong wrote:
> > On Sun, Oct 23, 2022 at 06:48:13AM +0000,yangx.jy@fujitsu.com wrote:
> >> generic/470 was original designed to verify mmap(MAP_SYNC) which is
> >> only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
> >> introduced to fix the inconsistent filesystem issue[2] but it make
> >> the test become not run because it doesn't support DAX. As Darrick
> >> mentioned[3], discarding the entire mapped range of scartch device
> >> can fix the issue as well, so I try to use blkdiscard -z instead.
> > That might be ok for the*other* dm-logwrites tests, but isn't the
> > fundamental problem here (generic/470, specifically) that device
> > mapper cannot run on top of pmem?
>
> Hi Darrick,
>
> With the change，I didn't find any failure when running generic/470 in loops.
> --------------------------------------------------------------
> [root@fedora35 xfstests-dev]# ./check generic/470
> FSTYP -- xfs (non-debug)
> PLATFORM -- Linux/x86_64 fedora35 6.1.0-rc1+ #37 SMP
> PREEMPT_DYNAMIC Fri Oct 21 19:04:57 CST 2022 MKFS_OPTIONS -- -f /dev/pmem0 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/pmem0 /mnt/scratch
>
> generic/470 6s
> Ran: generic/470
> Passed all 1 tests
> --------------------------------------------------------------
> Both dm-log-writes and PMEM support DAX so it's fine to verify
> mmap(MAP_SYNC) with the dm-log-writes device on top of PMEM.
>
> Did I miss something? Why do you think there is a fundamental problem here?
Nope, you're right. fsdax works fine, at least on these simple(r)
device mapper devices:
$ git grep -c dax drivers/md/
drivers/md/dm-core.h:1
drivers/md/dm-linear.c:19
drivers/md/dm-log-writes.c:19
drivers/md/dm-stripe.c:19
drivers/md/dm-table.c:18
drivers/md/dm-target.c:4
drivers/md/dm-writecache.c:7
drivers/md/dm.c:36
(Most notably, dm-logwrites :))
I'll go look at the test in the morning.
--D
> Best Regards,
> Xiao Yang
>
> >
> > --D
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-11-01 3:48 ` Darrick J. Wong
@ 2022-11-12 5:48 ` Yang, Xiao/杨 晓
0 siblings, 0 replies; 10+ messages in thread
From: Yang, Xiao/杨 晓 @ 2022-11-12 5:48 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: zlang, fstests, bfoster
On 2022/11/1 11:48, Darrick J. Wong wrote:
> On Sun, Oct 30, 2022 at 07:30:31AM +0000, yangx.jy@fujitsu.com wrote:
>> Hi Darrick,
>>
>> Ping, is there any feedback on the patch?
>>
>> Best Regards,
>> Xiao Yang
>>
>> -----Original Message-----
>> From: Yang, Xiao/杨 晓 <yangx.jy@fujitsu.com>
>> Sent: 2022年10月24日 15:16
>> To: Darrick J. Wong <djwong@kernel.org>
>> Cc: zlang@redhat.com; fstests@vger.kernel.org; bfoster@redhat.com
>> Subject: Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
>>
>> On 2022/10/24 12:09, Darrick J. Wong wrote:
>>> On Sun, Oct 23, 2022 at 06:48:13AM +0000,yangx.jy@fujitsu.com wrote:
>>>> generic/470 was original designed to verify mmap(MAP_SYNC) which is
>>>> only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
>>>> introduced to fix the inconsistent filesystem issue[2] but it make
>>>> the test become not run because it doesn't support DAX. As Darrick
>>>> mentioned[3], discarding the entire mapped range of scartch device
>>>> can fix the issue as well, so I try to use blkdiscard -z instead.
>>> That might be ok for the*other* dm-logwrites tests, but isn't the
>>> fundamental problem here (generic/470, specifically) that device
>>> mapper cannot run on top of pmem?
>>
>> Hi Darrick,
>>
>> With the change，I didn't find any failure when running generic/470 in loops.
>> --------------------------------------------------------------
>> [root@fedora35 xfstests-dev]# ./check generic/470
>> FSTYP -- xfs (non-debug)
>> PLATFORM -- Linux/x86_64 fedora35 6.1.0-rc1+ #37 SMP
>> PREEMPT_DYNAMIC Fri Oct 21 19:04:57 CST 2022 MKFS_OPTIONS -- -f /dev/pmem0 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/pmem0 /mnt/scratch
>>
>> generic/470 6s
>> Ran: generic/470
>> Passed all 1 tests
>> --------------------------------------------------------------
>> Both dm-log-writes and PMEM support DAX so it's fine to verify
>> mmap(MAP_SYNC) with the dm-log-writes device on top of PMEM.
>>
>> Did I miss something? Why do you think there is a fundamental problem here?
>
> Nope, you're right. fsdax works fine, at least on these simple(r)
> device mapper devices:
>
> $ git grep -c dax drivers/md/
> drivers/md/dm-core.h:1
> drivers/md/dm-linear.c:19
> drivers/md/dm-log-writes.c:19
> drivers/md/dm-stripe.c:19
> drivers/md/dm-table.c:18
> drivers/md/dm-target.c:4
> drivers/md/dm-writecache.c:7
> drivers/md/dm.c:36
>
> (Most notably, dm-logwrites :))
>
> I'll go look at the test in the morning.
Hi Darrick,
Do you have time to look at it?
I hope you can give me reviewed-by if you think this patch set is OK.
Best Regards,
Xiao Yang
>
> --D
>
>> Best Regards,
>> Xiao Yang
>>
>>>
>>> --D
>>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size
2022-10-23 6:48 [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size yangx.jy
2022-10-23 6:48 ` [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z yangx.jy
@ 2022-11-13 17:54 ` Darrick J. Wong
1 sibling, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2022-11-13 17:54 UTC (permalink / raw)
To: yangx.jy; +Cc: zlang, fstests, bfoster, Yasunori Gotou (Fujitsu)
On Sun, Oct 23, 2022 at 06:48:13AM +0000, yangx.jy@fujitsu.com wrote:
> It is unnecssary to always create a log-writes device based on
> the size of the entire underlying device.
>
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
> common/dmlogwrites | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 9fa1c977..d7b23cec 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -61,12 +61,20 @@ _require_log_writes_dax_mountopt()
>
> _log_writes_init()
> {
> - blkdev=$1
> + local blkdev=$1
> + local range=$2
Range is a length in ... bytes? The unit ought to be recorded in a
comment for this function.
With that fixed, this looks ok to me
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> + local BLK_DEV_SIZE
>
> [ -z "$blkdev" ] && _fail \
> "block dev must be specified for _log_writes_init"
>
> - local BLK_DEV_SIZE=`blockdev --getsz $blkdev`
> + if [ -z "$range" ]; then
> + BLK_DEV_SIZE=`blockdev --getsz $blkdev`
> + else
> + local blksz=`blockdev --getss $blkdev`
> + BLK_DEV_SIZE=$((range / blksz))
> + fi
> +
> LOGWRITES_NAME=logwrites-test
> LOGWRITES_DMDEV=/dev/mapper/$LOGWRITES_NAME
> LOGWRITES_TABLE="0 $BLK_DEV_SIZE log-writes $blkdev $LOGWRITES_DEV"
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-10-23 6:48 ` [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z yangx.jy
2022-10-24 4:09 ` Darrick J. Wong
@ 2022-11-13 17:57 ` Darrick J. Wong
2022-11-14 8:38 ` Yang, Xiao/杨 晓
1 sibling, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2022-11-13 17:57 UTC (permalink / raw)
To: yangx.jy; +Cc: zlang, fstests, bfoster, Yasunori Gotou (Fujitsu)
On Sun, Oct 23, 2022 at 06:48:13AM +0000, yangx.jy@fujitsu.com wrote:
> generic/470 was original designed to verify mmap(MAP_SYNC) which
> is only vaild to the DAX capable device(e.g. PMEM). Thin volume[1] was
> introduced to fix the inconsistent filesystem issue[2] but it make
> the test become not run because it doesn't support DAX. As Darrick
> mentioned[3], discarding the entire mapped range of scartch device
> can fix the issue as well, so I try to use blkdiscard -z instead.
>
> [1]: https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=fc5870da485aec0f9196a0f2bed32f73f6b2c664
> [2]: https://lore.kernel.org/fstests/20190227061529.GF16436@dastard/
> [3]: https://lore.kernel.org/linux-xfs/Y1NRNtToQTjs0Dbd@magnolia/T/#me0e77cb0ecd80bf4b5109e4433cb4863ae6e6727
>
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
> tests/generic/470 | 31 ++++++++++++-------------------
> 1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/tests/generic/470 b/tests/generic/470
> index f3407511..d2846a38 100755
> --- a/tests/generic/470
> +++ b/tests/generic/470
> @@ -15,38 +15,29 @@ _cleanup()
> {
> cd /
> _log_writes_cleanup
> - _dmthin_cleanup
> rm -f $tmp.*
> }
>
> # Import common functions.
> . ./common/filter
> -. ./common/dmthin
> . ./common/dmlogwrites
>
> # real QA test starts here
> _supported_fs generic
> -_require_scratch_nocheck
> +_require_scratch
> _require_no_logdev
> _require_log_writes_dax_mountopt "dax"
> -_require_dm_target thin-pool
> _require_xfs_io_command "mmap" "-S"
> _require_xfs_io_command "log_writes"
> +_require_command "$BLKDISCARD_PROG" blkdiscard
>
> -devsize=$((1024*1024*200 / 512)) # 200m phys/virt size
> -csize=$((1024*64 / 512)) # 64k cluster size
> -lowspace=$((1024*1024 / 512)) # 1m low space threshold
> -
> -# Use a thin device to provide deterministic discard behavior. Discards are used
> -# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
> -_dmthin_init $devsize $devsize $csize $lowspace
> +RANGE=$((512 * 1024 * 1024)) # 512 MiB
> +LEN=$((1024 * 1024)) # 1 MiB
>
> -_log_writes_init $DMTHIN_VOL_DEV
> +_log_writes_init $SCRATCH_DEV $RANGE
> _log_writes_mkfs >> $seqres.full 2>&1
> _log_writes_mount -o dax
>
> -LEN=$((1024 * 1024)) # 1 MiB
> -
> $XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> -c "log_writes -d $LOGWRITES_NAME -m preunmap" \
> -f $SCRATCH_MNT/test
> @@ -54,14 +45,16 @@ $XFS_IO_PROG -t -c "truncate $LEN" -c "mmap -S 0 $LEN" -c "mwrite 0 $LEN" \
> # Unmount the scratch dir and tear down the log writes target
> _log_writes_unmount
> _log_writes_remove
> -_dmthin_check_fs
> +_check_scratch_fs
>
> -# destroy previous filesystem so we can be sure our rebuild works
> -_mkfs_dev $DMTHIN_VOL_DEV >> $seqres.full 2>&1
> +# Discard the mapped range of scratch device and destroy previous filesystem
> +# so we can be sure our rebuild works
> +$BLKDISCARD_PROG -fzl $RANGE $SCRATCH_DEV >> $seqres.full 2>&1
You used -z, which means the comment should say "Forcibly zero the
mapped range of the scratch device..." and note that blkdiscard -z will
fall back to writing buffers of zeroes if the device doesn't support
WRITE SAME with zeroes.
I think the logic in here looks good though.
--D
> +_scratch_mkfs >> $seqres.full 2>&1
>
> # check pre-unmap state
> -_log_writes_replay_log preunmap $DMTHIN_VOL_DEV
> -_dmthin_mount
> +_log_writes_replay_log preunmap $SCRATCH_DEV
> +_scratch_mount
>
> # We should see $SCRATCH_MNT/test as having 1 MiB in block allocations
> du -sh $SCRATCH_MNT/test | _filter_scratch | _filter_spaces
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z
2022-11-13 17:57 ` Darrick J. Wong
@ 2022-11-14 8:38 ` Yang, Xiao/杨 晓
0 siblings, 0 replies; 10+ messages in thread
From: Yang, Xiao/杨 晓 @ 2022-11-14 8:38 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: zlang, fstests, bfoster, Yasunori Gotou (Fujitsu)
On 2022/11/14 1:57, Darrick J. Wong wrote:
> You used -z, which means the comment should say "Forcibly zero the
> mapped range of the scratch device..." and note that blkdiscard -z will
> fall back to writing buffers of zeroes if the device doesn't support
> WRITE SAME with zeroes.
>
> I think the logic in here looks good though.
>
> --D
Hi Darrick,
Thanks a lot for your comment.
I have sent the v2 patch set[1][2] as you suggested. I hope you can
review them again.
[1]
https://lore.kernel.org/fstests/1668414903-13-1-git-send-email-yangx.jy@fujitsu.com/T/#t
[2]
https://lore.kernel.org/fstests/1668414903-13-2-git-send-email-yangx.jy@fujitsu.com/T/#u
Best Regards,
Xiao Yang
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-14 8:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-23 6:48 [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size yangx.jy
2022-10-23 6:48 ` [PATCH RESEND 2/2] generic/470: Replace thin volume with blkdiscard -z yangx.jy
2022-10-24 4:09 ` Darrick J. Wong
2022-10-24 7:15 ` Yang, Xiao/杨 晓
2022-10-30 7:30 ` yangx.jy
2022-11-01 3:48 ` Darrick J. Wong
2022-11-12 5:48 ` Yang, Xiao/杨 晓
2022-11-13 17:57 ` Darrick J. Wong
2022-11-14 8:38 ` Yang, Xiao/杨 晓
2022-11-13 17:54 ` [PATCH RESEND 1/2] common/dmlogwrites: Extend _log_writes_init() to accept the specified size Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).