diff mbox

generic: test dm-thin running out of data space vs concurrent discard

Message ID 20180629165738.8106-1-zlang@redhat.com (mailing list archive)
State Superseded
Headers show

Commit Message

Zorro Lang June 29, 2018, 4:57 p.m. UTC
If a user constructs a test that loops repeatedly over below steps
on dm-thin, block allocation can fail due to discards not having
completed yet (Fixed by a685557 dm thin: handle running out of data
space vs concurrent discard):
1) fill thin device via filesystem file
2) remove file
3) fstrim

And this maybe cause a deadlock (fast device likes ramdisk can help
a lot) when racing a fstrim with a filesystem (XFS) shutdown. (Fixed
by 8c81dd46ef3c Force log to disk before reading the AGF during a
fstrim)

This case can reproduce both two bugs if they're not fixed. If only
the dm-thin bug is fixed, then the test will pass. If only the fs
bug is fixed, then the test will fail. If both of bugs aren't fixed,
the test will hang.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

If both of two bugs aren't fixed, a loop device base on tmpfs can help
reproduce the XFS deadlock:
1) mount -t tmpfs tmpfs /tmp
2) dd if=/dev/zero of=/tmp/test.img bs=1M count=100
3) losetup /dev/loop0 /tmp/test.img
4) use /dev/loop0 to be SCRATCH_DEV, run this case. The test will hang there.

Ramdisk can help trigger the race. Maybe NVME device can help too. But it's
hard to reproduce on general disk.

If the XFS bug is fixed, above steps can reproduce dm-thin bug, the test
will fail.

Unfortunately, if the dm-thin bug is fixed, then this case can't reproduce
the XFS bug singly.

Thanks,
Zorro

 tests/generic/499     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/499.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 88 insertions(+)
 create mode 100755 tests/generic/499
 create mode 100644 tests/generic/499.out

Comments

Carlos Maiolino July 2, 2018, 9:27 a.m. UTC | #1
On Sat, Jun 30, 2018 at 12:57:38AM +0800, Zorro Lang wrote:
> If a user constructs a test that loops repeatedly over below steps
> on dm-thin, block allocation can fail due to discards not having
> completed yet (Fixed by a685557 dm thin: handle running out of data
> space vs concurrent discard):
> 1) fill thin device via filesystem file
> 2) remove file
> 3) fstrim
> 
> And this maybe cause a deadlock (fast device likes ramdisk can help
> a lot) when racing a fstrim with a filesystem (XFS) shutdown. (Fixed
> by 8c81dd46ef3c Force log to disk before reading the AGF during a
> fstrim)
> 
> This case can reproduce both two bugs if they're not fixed. If only
> the dm-thin bug is fixed, then the test will pass. If only the fs
> bug is fixed, then the test will fail. If both of bugs aren't fixed,
> the test will hang.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> If both of two bugs aren't fixed, a loop device base on tmpfs can help
> reproduce the XFS deadlock:
> 1) mount -t tmpfs tmpfs /tmp
> 2) dd if=/dev/zero of=/tmp/test.img bs=1M count=100
> 3) losetup /dev/loop0 /tmp/test.img
> 4) use /dev/loop0 to be SCRATCH_DEV, run this case. The test will hang there.

Particularly, I could never reproduce this bug on spindles or SSDs, and I
believe many (if not most) people run xfstests on commodity hardware, not on
very fast disks, and the test doesn't reproduce the bug 100% of the times when
running on slow disks, so, unless the default for the test is to run it using
ramdisks, the test is useless IMHO.

> 
> Ramdisk can help trigger the race. Maybe NVME device can help too. But it's
> hard to reproduce on general disk.
> 

I didn't test it on NVME, so I can't tell =/

> If the XFS bug is fixed, above steps can reproduce dm-thin bug, the test
> will fail.
> 
> Unfortunately, if the dm-thin bug is fixed, then this case can't reproduce
> the XFS bug singly.
> 
> Thanks,
> Zorro
> 
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 499
> +#
> +# Race test running out of data space with concurrent discard operation on
> +# dm-thin.
> +#
> +# If a user constructs a test that loops repeatedly over below steps on
> +# dm-thin, block allocation can fail due to discards not having completed
> +# yet (Fixed by a685557 dm thin: handle running out of data space vs
> +# concurrent discard):
> +# 1) fill thin device via filesystem file
> +# 2) remove file
> +# 3) fstrim
> +#
> +# And this maybe cause a deadlock when racing a fstrim with a filesystem
> +# (XFS) shutdown. (Fixed by 8c81dd46ef3c Force log to disk before reading
> +# the AGF during a fstrim)
> +


> +# There're two bugs at here, one is dm-thin bug, the other is filesystem
> +# (XFS especially) bug. The dm-thin bug can't handle running out of data
> +# space with concurrent discard well. Then the dm-thin bug cause fs unmount
> +# hang when racing a fstrim with a filesystem shutdown.
> +#
> +# If both of two bugs haven't been fixed, below test maybe cause deadlock.
> +# Else if the fs bug has been fixed, but the dm-thin bug hasn't. below test
> +# will cause the test fail (no deadlock).
> +# Else the test will pass.

The test looks mostly ok, despite the fact I believe this should run on a
ramdisk by default (or not run, if $SCRATCH_DEV is not a ramdisk)
Zorro Lang July 2, 2018, 10:28 a.m. UTC | #2
On Mon, Jul 02, 2018 at 11:27:11AM +0200, Carlos Maiolino wrote:
> On Sat, Jun 30, 2018 at 12:57:38AM +0800, Zorro Lang wrote:
> > If a user constructs a test that loops repeatedly over below steps
> > on dm-thin, block allocation can fail due to discards not having
> > completed yet (Fixed by a685557 dm thin: handle running out of data
> > space vs concurrent discard):
> > 1) fill thin device via filesystem file
> > 2) remove file
> > 3) fstrim
> > 
> > And this maybe cause a deadlock (fast device likes ramdisk can help
> > a lot) when racing a fstrim with a filesystem (XFS) shutdown. (Fixed
> > by 8c81dd46ef3c Force log to disk before reading the AGF during a
> > fstrim)
> > 
> > This case can reproduce both two bugs if they're not fixed. If only
> > the dm-thin bug is fixed, then the test will pass. If only the fs
> > bug is fixed, then the test will fail. If both of bugs aren't fixed,
> > the test will hang.
> > 
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > ---
> > 
> > Hi,
> > 
> > If both of two bugs aren't fixed, a loop device base on tmpfs can help
> > reproduce the XFS deadlock:
> > 1) mount -t tmpfs tmpfs /tmp
> > 2) dd if=/dev/zero of=/tmp/test.img bs=1M count=100
> > 3) losetup /dev/loop0 /tmp/test.img
> > 4) use /dev/loop0 to be SCRATCH_DEV, run this case. The test will hang there.
> 
> Particularly, I could never reproduce this bug on spindles or SSDs, and I
> believe many (if not most) people run xfstests on commodity hardware, not on
> very fast disks, and the test doesn't reproduce the bug 100% of the times when
> running on slow disks, so, unless the default for the test is to run it using
> ramdisks, the test is useless IMHO.

As a racing test, I think there's not 100% reproducible case. This case
already can cover this issue in some conditions.

> 
> > 
> > Ramdisk can help trigger the race. Maybe NVME device can help too. But it's
> > hard to reproduce on general disk.
> > 
> 
> I didn't test it on NVME, so I can't tell =/

I didn't try NVME and SSD. From my test, if the underlying SCRATCH_DEV support
fstrim, the case can reproduce this bug.

For example:

If I create a device by:
# modprobe scsi_debug dev_size_mb=100
Then I can't reproduce this bug.

If I create a device by
# modprobe scsi_debug lbpu=1 lbpws=1 dev_size_mb=100
Then the bug is reproducible:
# ./check generic/499
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/x86_64 xxxx 3.10.0-915.el7.x86_64
MKFS_OPTIONS  -- -f -bsize=4096 /dev/sde
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sde /mnt/scratch

generic/499 2s ... [failed, exit status 1]- output mismatch (see /root/git/xfstests-zlang/results//generic/499.out.bad)
    --- tests/generic/499.out   2018-06-29 10:38:58.965827495 -0400
    +++ /root/git/xfstests-zlang/results//generic/499.out.bad   2018-07-02 06:20:34.841313041 -0400
    @@ -1,2 +1,106 @@
     QA output created by 499
    -Silence is golden
    +fstrim: /mnt/scratch: FITRIM ioctl failed: Input/output error
    +fstrim: cannot open /mnt/scratch: Input/output error
    +fstrim: cannot open /mnt/scratch: Input/output error
    +fstrim: cannot open /mnt/scratch: Input/output error
    +fstrim: cannot open /mnt/scratch: Input/output error
    ...
    (Run 'diff -u tests/generic/499.out /root/git/xfstests-zlang/results//generic/499.out.bad'  to see the entire diff)
Ran: generic/499
Failures: generic/499
Failed 1 of 1 tests

Thanks,
Zorro

> 
> > If the XFS bug is fixed, above steps can reproduce dm-thin bug, the test
> > will fail.
> > 
> > Unfortunately, if the dm-thin bug is fixed, then this case can't reproduce
> > the XFS bug singly.
> > 
> > Thanks,
> > Zorro
> > 
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2018 Red Hat Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test 499
> > +#
> > +# Race test running out of data space with concurrent discard operation on
> > +# dm-thin.
> > +#
> > +# If a user constructs a test that loops repeatedly over below steps on
> > +# dm-thin, block allocation can fail due to discards not having completed
> > +# yet (Fixed by a685557 dm thin: handle running out of data space vs
> > +# concurrent discard):
> > +# 1) fill thin device via filesystem file
> > +# 2) remove file
> > +# 3) fstrim
> > +#
> > +# And this maybe cause a deadlock when racing a fstrim with a filesystem
> > +# (XFS) shutdown. (Fixed by 8c81dd46ef3c Force log to disk before reading
> > +# the AGF during a fstrim)
> > +
> 
> 
> > +# There're two bugs at here, one is dm-thin bug, the other is filesystem
> > +# (XFS especially) bug. The dm-thin bug can't handle running out of data
> > +# space with concurrent discard well. Then the dm-thin bug cause fs unmount
> > +# hang when racing a fstrim with a filesystem shutdown.
> > +#
> > +# If both of two bugs haven't been fixed, below test maybe cause deadlock.
> > +# Else if the fs bug has been fixed, but the dm-thin bug hasn't. below test
> > +# will cause the test fail (no deadlock).
> > +# Else the test will pass.
> 
> The test looks mostly ok, despite the fact I believe this should run on a
> ramdisk by default (or not run, if $SCRATCH_DEV is not a ramdisk)
> 
> -- 
> Carlos
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan July 4, 2018, 1:04 p.m. UTC | #3
On Sat, Jun 30, 2018 at 12:57:38AM +0800, Zorro Lang wrote:
> If a user constructs a test that loops repeatedly over below steps
> on dm-thin, block allocation can fail due to discards not having
> completed yet (Fixed by a685557 dm thin: handle running out of data
> space vs concurrent discard):
> 1) fill thin device via filesystem file
> 2) remove file
> 3) fstrim
> 
> And this maybe cause a deadlock (fast device likes ramdisk can help
> a lot) when racing a fstrim with a filesystem (XFS) shutdown. (Fixed
> by 8c81dd46ef3c Force log to disk before reading the AGF during a
> fstrim)
> 
> This case can reproduce both two bugs if they're not fixed. If only
> the dm-thin bug is fixed, then the test will pass. If only the fs
> bug is fixed, then the test will fail. If both of bugs aren't fixed,
> the test will hang.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> If both of two bugs aren't fixed, a loop device base on tmpfs can help
> reproduce the XFS deadlock:
> 1) mount -t tmpfs tmpfs /tmp
> 2) dd if=/dev/zero of=/tmp/test.img bs=1M count=100
> 3) losetup /dev/loop0 /tmp/test.img
> 4) use /dev/loop0 to be SCRATCH_DEV, run this case. The test will hang there.
> 
> Ramdisk can help trigger the race. Maybe NVME device can help too. But it's
> hard to reproduce on general disk.
> 
> If the XFS bug is fixed, above steps can reproduce dm-thin bug, the test
> will fail.
> 
> Unfortunately, if the dm-thin bug is fixed, then this case can't reproduce
> the XFS bug singly.
> 
> Thanks,
> Zorro
> 
>  tests/generic/499     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/499.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 88 insertions(+)
>  create mode 100755 tests/generic/499
>  create mode 100644 tests/generic/499.out
> 
> diff --git a/tests/generic/499 b/tests/generic/499
> new file mode 100755
> index 00000000..24adfc3a
> --- /dev/null
> +++ b/tests/generic/499
> @@ -0,0 +1,85 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Red Hat Inc.  All Rights Reserved.
> +#
> +# FS QA Test 499
> +#
> +# Race test running out of data space with concurrent discard operation on
> +# dm-thin.
> +#
> +# If a user constructs a test that loops repeatedly over below steps on
> +# dm-thin, block allocation can fail due to discards not having completed
> +# yet (Fixed by a685557 dm thin: handle running out of data space vs
> +# concurrent discard):
> +# 1) fill thin device via filesystem file
> +# 2) remove file
> +# 3) fstrim
> +#
> +# And this maybe cause a deadlock when racing a fstrim with a filesystem
> +# (XFS) shutdown. (Fixed by 8c81dd46ef3c Force log to disk before reading
> +# the AGF during a fstrim)
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +	_dmthin_cleanup
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/dmthin
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch_nocheck
> +_require_dm_target thin-pool

Need _require_command "$FSTRIM_PROG" fstrim

> +
> +# Create a thin pool and a *slightly smaller* thin volume, it's helpful
> +# to reproduce the bug
> +BACKING_SIZE=$((50 * 1024 * 1024 / 512))	# 50M
> +VIRTUAL_SIZE=$((BACKING_SIZE + 1024))		# 50M + 1k
> +CLUSTER_SIZE=$((64 * 1024 / 512))		# 64K
> +
> +_dmthin_init $BACKING_SIZE $VIRTUAL_SIZE $CLUSTER_SIZE 0
> +_dmthin_set_fail
> +_mkfs_dev $DMTHIN_VOL_DEV
> +_dmthin_mount
> +
> +# There're two bugs at here, one is dm-thin bug, the other is filesystem
> +# (XFS especially) bug. The dm-thin bug can't handle running out of data
> +# space with concurrent discard well. Then the dm-thin bug cause fs unmount
> +# hang when racing a fstrim with a filesystem shutdown.
> +#
> +# If both of two bugs haven't been fixed, below test maybe cause deadlock.
> +# Else if the fs bug has been fixed, but the dm-thin bug hasn't. below test
> +# will cause the test fail (no deadlock).
> +# Else the test will pass.
> +for ((i=0; i<100; i++)); do
> +	$XFS_IO_PROG -f -c "pwrite -b 64k 0 100M" \
> +		$SCRATCH_MNT/testfile &>/dev/null
> +	rm -f $SCRATCH_MNT/testfile
> +	$FSTRIM_PROG $SCRATCH_MNT
> +done

I think it's fine to run this test on non-ramdisk devices, even though
it's hard to reproduce the bug there, as it's good to have more test
coverage. But is it really need looping for 100 times? Test time could
be reduced if we could run for less loops.

Thanks,
Eryu

> +
> +_dmthin_check_fs
> +_dmthin_cleanup
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/499.out b/tests/generic/499.out
> new file mode 100644
> index 00000000..c363e684
> --- /dev/null
> +++ b/tests/generic/499.out
> @@ -0,0 +1,2 @@
> +QA output created by 499
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 83a6fdab..bbeac4af 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -501,3 +501,4 @@
>  496 auto quick swap
>  497 auto quick swap collapse
>  498 auto quick log
> +499 auto thin trim
> -- 
> 2.14.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/generic/499 b/tests/generic/499
new file mode 100755
index 00000000..24adfc3a
--- /dev/null
+++ b/tests/generic/499
@@ -0,0 +1,85 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Red Hat Inc.  All Rights Reserved.
+#
+# FS QA Test 499
+#
+# Race test running out of data space with concurrent discard operation on
+# dm-thin.
+#
+# If a user constructs a test that loops repeatedly over below steps on
+# dm-thin, block allocation can fail due to discards not having completed
+# yet (Fixed by a685557 dm thin: handle running out of data space vs
+# concurrent discard):
+# 1) fill thin device via filesystem file
+# 2) remove file
+# 3) fstrim
+#
+# And this maybe cause a deadlock when racing a fstrim with a filesystem
+# (XFS) shutdown. (Fixed by 8c81dd46ef3c Force log to disk before reading
+# the AGF during a fstrim)
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	_dmthin_cleanup
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/dmthin
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch_nocheck
+_require_dm_target thin-pool
+
+# Create a thin pool and a *slightly smaller* thin volume, it's helpful
+# to reproduce the bug
+BACKING_SIZE=$((50 * 1024 * 1024 / 512))	# 50M
+VIRTUAL_SIZE=$((BACKING_SIZE + 1024))		# 50M + 1k
+CLUSTER_SIZE=$((64 * 1024 / 512))		# 64K
+
+_dmthin_init $BACKING_SIZE $VIRTUAL_SIZE $CLUSTER_SIZE 0
+_dmthin_set_fail
+_mkfs_dev $DMTHIN_VOL_DEV
+_dmthin_mount
+
+# There're two bugs at here, one is dm-thin bug, the other is filesystem
+# (XFS especially) bug. The dm-thin bug can't handle running out of data
+# space with concurrent discard well. Then the dm-thin bug cause fs unmount
+# hang when racing a fstrim with a filesystem shutdown.
+#
+# If both of two bugs haven't been fixed, below test maybe cause deadlock.
+# Else if the fs bug has been fixed, but the dm-thin bug hasn't. below test
+# will cause the test fail (no deadlock).
+# Else the test will pass.
+for ((i=0; i<100; i++)); do
+	$XFS_IO_PROG -f -c "pwrite -b 64k 0 100M" \
+		$SCRATCH_MNT/testfile &>/dev/null
+	rm -f $SCRATCH_MNT/testfile
+	$FSTRIM_PROG $SCRATCH_MNT
+done
+
+_dmthin_check_fs
+_dmthin_cleanup
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/499.out b/tests/generic/499.out
new file mode 100644
index 00000000..c363e684
--- /dev/null
+++ b/tests/generic/499.out
@@ -0,0 +1,2 @@ 
+QA output created by 499
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 83a6fdab..bbeac4af 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -501,3 +501,4 @@ 
 496 auto quick swap
 497 auto quick swap collapse
 498 auto quick log
+499 auto thin trim