fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs: add a test for umount racing mount
@ 2020-07-10  0:55 Boris Burkov
  2020-07-10 14:35 ` Josef Bacik
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Burkov @ 2020-07-10  0:55 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs

Test if dirtying many inodes, which delays the umount in `evict_inodes`,
then umounting and quickly mounting again causes the mount to fail.

This race is fixed by the patch:
"btrfs: fix mount failure caused by race with umount"

Signed-off-by: Boris Burkov <boris@bur.io>
---
 tests/btrfs/215     | 52 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/215.out |  2 ++
 tests/btrfs/group   |  1 +
 3 files changed, 55 insertions(+)
 create mode 100755 tests/btrfs/215
 create mode 100644 tests/btrfs/215.out

diff --git a/tests/btrfs/215 b/tests/btrfs/215
new file mode 100755
index 00000000..b142c2d6
--- /dev/null
+++ b/tests/btrfs/215
@@ -0,0 +1,52 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 YOUR NAME HERE.  All Rights Reserved.
+#
+# FS QA Test 215
+#
+# Evicting dirty inodes can take a long time during umount.
+# Check that a new mount racing with such a delayed umount succeeds.
+#
+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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+for i in $(seq 0 500)
+	do
+		  dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
+		  done
+		  _scratch_unmount&
+		  _scratch_mount
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
new file mode 100644
index 00000000..0a11773b
--- /dev/null
+++ b/tests/btrfs/215.out
@@ -0,0 +1,2 @@
+QA output created by 215
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index 505665b5..dda0763e 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -217,3 +217,4 @@
 212 auto balance dangerous
 213 auto balance dangerous
 214 auto quick send snapshot
+215 auto quick
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] btrfs: add a test for umount racing mount
  2020-07-10  0:55 [PATCH] btrfs: add a test for umount racing mount Boris Burkov
@ 2020-07-10 14:35 ` Josef Bacik
  2020-07-10 17:18   ` [PATCH v2] generic: " Boris Burkov
  0 siblings, 1 reply; 11+ messages in thread
From: Josef Bacik @ 2020-07-10 14:35 UTC (permalink / raw)
  To: Boris Burkov, fstests; +Cc: linux-btrfs

On 7/9/20 8:55 PM, Boris Burkov wrote:
> Test if dirtying many inodes, which delays the umount in `evict_inodes`,
> then umounting and quickly mounting again causes the mount to fail.
> 
> This race is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>   tests/btrfs/215     | 52 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/215.out |  2 ++
>   tests/btrfs/group   |  1 +
>   3 files changed, 55 insertions(+)
>   create mode 100755 tests/btrfs/215
>   create mode 100644 tests/btrfs/215.out
> 
> diff --git a/tests/btrfs/215 b/tests/btrfs/215
> new file mode 100755
> index 00000000..b142c2d6
> --- /dev/null
> +++ b/tests/btrfs/215
> @@ -0,0 +1,52 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 YOUR NAME HERE.  All Rights Reserved.

Facebook should go in "YOUR NAME HERE".

> +#
> +# FS QA Test 215
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic

This isn't btrfs specific, so I'd put it in tests/generic/<whatever>

> +_supported_os Linux
> +_require_test
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +	do
> +		  dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
> +		  done
> +		  _scratch_unmount&
> +		  _scratch_mount
> +

Looks like your tabbing got carried away here, and add a space between 
_scratch_unmount and the &.

> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/215.out b/tests/btrfs/215.out
> new file mode 100644
> index 00000000..0a11773b
> --- /dev/null
> +++ b/tests/btrfs/215.out
> @@ -0,0 +1,2 @@
> +QA output created by 215
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index 505665b5..dda0763e 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -217,3 +217,4 @@
>   212 auto balance dangerous
>   213 auto balance dangerous
>   214 auto quick send snapshot
> +215 auto quick

Spacing appears to be off here too.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] generic: add a test for umount racing mount
  2020-07-10 14:35 ` Josef Bacik
@ 2020-07-10 17:18   ` Boris Burkov
  2020-07-10 17:52     ` Josef Bacik
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Boris Burkov @ 2020-07-10 17:18 UTC (permalink / raw)
  To: Josef Bacik, fstests; +Cc: linux-btrfs

Test if dirtying many inodes (which can delay umount) then
unmounting and quickly mounting again causes the mount to fail.

A race, which breaks the test in btrfs, is fixed by the patch:
"btrfs: fix mount failure caused by race with umount"

Signed-off-by: Boris Burkov <boris@bur.io>
---
 tests/generic/603     | 52 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/603.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 55 insertions(+)
 create mode 100755 tests/generic/603
 create mode 100644 tests/generic/603.out

diff --git a/tests/generic/603 b/tests/generic/603
new file mode 100755
index 00000000..e67899cb
--- /dev/null
+++ b/tests/generic/603
@@ -0,0 +1,52 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Facebook  All Rights Reserved.
+#
+# FS QA Test 603
+#
+# Evicting dirty inodes can take a long time during umount.
+# Check that a new mount racing with such a delayed umount succeeds.
+#
+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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+for i in $(seq 0 500)
+do
+	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
+done
+_scratch_unmount &
+_scratch_mount
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/603.out b/tests/generic/603.out
new file mode 100644
index 00000000..6810da89
--- /dev/null
+++ b/tests/generic/603.out
@@ -0,0 +1,2 @@
+QA output created by 603
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index d9ab9a31..c0ace35b 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -605,3 +605,4 @@
 600 auto quick quota
 601 auto quick quota
 602 auto quick encrypt
+603 auto quick
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generic: add a test for umount racing mount
  2020-07-10 17:18   ` [PATCH v2] generic: " Boris Burkov
@ 2020-07-10 17:52     ` Josef Bacik
  2020-07-12 11:37     ` Zorro Lang
  2020-07-19 17:14     ` [PATCH v2] " Eryu Guan
  2 siblings, 0 replies; 11+ messages in thread
From: Josef Bacik @ 2020-07-10 17:52 UTC (permalink / raw)
  To: Boris Burkov, fstests; +Cc: linux-btrfs

On 7/10/20 1:18 PM, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generic: add a test for umount racing mount
  2020-07-10 17:18   ` [PATCH v2] generic: " Boris Burkov
  2020-07-10 17:52     ` Josef Bacik
@ 2020-07-12 11:37     ` Zorro Lang
  2020-07-13 20:46       ` [PATCH v3] " Boris Burkov
  2020-07-19 17:14     ` [PATCH v2] " Eryu Guan
  2 siblings, 1 reply; 11+ messages in thread
From: Zorro Lang @ 2020-07-12 11:37 UTC (permalink / raw)
  To: Boris Burkov; +Cc: Josef Bacik, fstests, linux-btrfs

On Fri, Jul 10, 2020 at 10:18:36AM -0700, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  tests/generic/603     | 52 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/603.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 55 insertions(+)
>  create mode 100755 tests/generic/603
>  create mode 100644 tests/generic/603.out
> 
> diff --git a/tests/generic/603 b/tests/generic/603
> new file mode 100755
> index 00000000..e67899cb
> --- /dev/null
> +++ b/tests/generic/603
> @@ -0,0 +1,52 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Facebook  All Rights Reserved.
> +#
> +# FS QA Test 603
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_test

It should be _require_scratch, due to you never use TEST_DIR below, but you
used $SCRATCH_MNT.

> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +do
> +	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
> +done
> +_scratch_unmount &

I have a question, can we make sure the this umount operation won't affect
next running case?

> +_scratch_mount
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/603.out b/tests/generic/603.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/603.out
> @@ -0,0 +1,2 @@
> +QA output created by 603
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..c0ace35b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
>  600 auto quick quota
>  601 auto quick quota
>  602 auto quick encrypt
> +603 auto quick
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] generic: add a test for umount racing mount
  2020-07-12 11:37     ` Zorro Lang
@ 2020-07-13 20:46       ` Boris Burkov
  2020-07-14  5:21         ` Zorro Lang
  2020-07-19 17:18         ` Eryu Guan
  0 siblings, 2 replies; 11+ messages in thread
From: Boris Burkov @ 2020-07-13 20:46 UTC (permalink / raw)
  To: Zorro Lang, fstests; +Cc: linux-btrfs

Test if dirtying many inodes (which can delay umount) then
unmounting and quickly mounting again causes the mount to fail.

A race, which breaks the test in btrfs, is fixed by the patch:
"btrfs: fix mount failure caused by race with umount"

Signed-off-by: Boris Burkov <boris@bur.io>
---
 tests/generic/603     | 53 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/603.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 56 insertions(+)
 create mode 100755 tests/generic/603
 create mode 100644 tests/generic/603.out

diff --git a/tests/generic/603 b/tests/generic/603
new file mode 100755
index 00000000..8e9a80e6
--- /dev/null
+++ b/tests/generic/603
@@ -0,0 +1,53 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Facebook  All Rights Reserved.
+#
+# FS QA Test 603
+#
+# Evicting dirty inodes can take a long time during umount.
+# Check that a new mount racing with such a delayed umount succeeds.
+#
+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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+for i in $(seq 0 500)
+do
+	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
+done
+_scratch_unmount &
+_scratch_mount
+wait
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/603.out b/tests/generic/603.out
new file mode 100644
index 00000000..6810da89
--- /dev/null
+++ b/tests/generic/603.out
@@ -0,0 +1,2 @@
+QA output created by 603
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index d9ab9a31..c0ace35b 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -605,3 +605,4 @@
 600 auto quick quota
 601 auto quick quota
 602 auto quick encrypt
+603 auto quick
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] generic: add a test for umount racing mount
  2020-07-13 20:46       ` [PATCH v3] " Boris Burkov
@ 2020-07-14  5:21         ` Zorro Lang
  2020-07-19 17:18         ` Eryu Guan
  1 sibling, 0 replies; 11+ messages in thread
From: Zorro Lang @ 2020-07-14  5:21 UTC (permalink / raw)
  To: Boris Burkov; +Cc: fstests, linux-btrfs

On Mon, Jul 13, 2020 at 01:46:39PM -0700, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---

If Eryu won't tend to change dd to XFS_IO_PROG, this version is good to me:)

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

Thanks,
Zorro

>  tests/generic/603     | 53 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/603.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 56 insertions(+)
>  create mode 100755 tests/generic/603
>  create mode 100644 tests/generic/603.out
> 
> diff --git a/tests/generic/603 b/tests/generic/603
> new file mode 100755
> index 00000000..8e9a80e6
> --- /dev/null
> +++ b/tests/generic/603
> @@ -0,0 +1,53 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Facebook  All Rights Reserved.
> +#
> +# FS QA Test 603
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +do
> +	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
> +done
> +_scratch_unmount &
> +_scratch_mount
> +wait
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/603.out b/tests/generic/603.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/603.out
> @@ -0,0 +1,2 @@
> +QA output created by 603
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..c0ace35b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
>  600 auto quick quota
>  601 auto quick quota
>  602 auto quick encrypt
> +603 auto quick
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generic: add a test for umount racing mount
  2020-07-10 17:18   ` [PATCH v2] generic: " Boris Burkov
  2020-07-10 17:52     ` Josef Bacik
  2020-07-12 11:37     ` Zorro Lang
@ 2020-07-19 17:14     ` Eryu Guan
  2 siblings, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2020-07-19 17:14 UTC (permalink / raw)
  To: Boris Burkov; +Cc: Josef Bacik, fstests, linux-btrfs, Amir Goldstein

[ cc'ed Amir for failure on overlayfs ]

On Fri, Jul 10, 2020 at 10:18:36AM -0700, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  tests/generic/604     | 52 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/604.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 55 insertions(+)
>  create mode 100755 tests/generic/604
>  create mode 100644 tests/generic/604.out
> 
> diff --git a/tests/generic/604 b/tests/generic/604
> new file mode 100755
> index 00000000..e67899cb
> --- /dev/null
> +++ b/tests/generic/604
> @@ -0,0 +1,52 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Facebook  All Rights Reserved.
> +#
> +# FS QA Test 604
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_test

Test takes use of scratch dev, but required test dev here.

> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +do
> +	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
> +done

The kernel patch describes that it only needs to make a bunch of inodes
dirty, so is it really necessary to write 500M data to the fs? Does
writing less files work (e.g. 50)? Or does writing less data work (e.g.
4k file)?

Also, fstests perfers code style like

for i in $(seq 0 500); do
	$XFS_IO_PROG -c "pwrite 0 1M" $SCRATCH_MNT/$i >/dev/null 2>&1
done

> +_scratch_unmount &
> +_scratch_mount

xfs and ext4 both passed this test, but overlayfs always fails the test.
I'm not sure if this is a valid test for overlay? Or it's just that
overlayfs should be fixed? Amir, any comments here?

Thanks,
Eryu

> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/604.out b/tests/generic/604.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/604.out
> @@ -0,0 +1,2 @@
> +QA output created by 604
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..c0ace35b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
>  601 auto quick quota
>  602 auto quick encrypt
>  603 auto quick quota
> +604 auto quick
> -- 
> 2.24.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] generic: add a test for umount racing mount
  2020-07-13 20:46       ` [PATCH v3] " Boris Burkov
  2020-07-14  5:21         ` Zorro Lang
@ 2020-07-19 17:18         ` Eryu Guan
  2020-07-20 19:05           ` [PATCH v4] " Boris Burkov
  1 sibling, 1 reply; 11+ messages in thread
From: Eryu Guan @ 2020-07-19 17:18 UTC (permalink / raw)
  To: Boris Burkov; +Cc: Zorro Lang, fstests, linux-btrfs

On Mon, Jul 13, 2020 at 01:46:39PM -0700, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  tests/generic/603     | 53 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/603.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 56 insertions(+)
>  create mode 100755 tests/generic/603
>  create mode 100644 tests/generic/603.out
> 
> diff --git a/tests/generic/603 b/tests/generic/603
> new file mode 100755
> index 00000000..8e9a80e6
> --- /dev/null
> +++ b/tests/generic/603
> @@ -0,0 +1,53 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Facebook  All Rights Reserved.
> +#
> +# FS QA Test 603
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch

Ah, it's fixed in v3, so ignore my first comment in the reply to v2 :)

Thanks,
Eryu

> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +do
> +	dd if=/dev/zero of="$SCRATCH_MNT/$i" bs=1M count=1 > /dev/null 2>&1
> +done
> +_scratch_unmount &
> +_scratch_mount
> +wait
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/603.out b/tests/generic/603.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/603.out
> @@ -0,0 +1,2 @@
> +QA output created by 603
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..c0ace35b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
>  600 auto quick quota
>  601 auto quick quota
>  602 auto quick encrypt
> +603 auto quick
> -- 
> 2.24.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4] generic: add a test for umount racing mount
  2020-07-19 17:18         ` Eryu Guan
@ 2020-07-20 19:05           ` Boris Burkov
  2020-07-21  5:03             ` Zorro Lang
  0 siblings, 1 reply; 11+ messages in thread
From: Boris Burkov @ 2020-07-20 19:05 UTC (permalink / raw)
  To: Eryu Guan, fstests; +Cc: linux-btrfs

Test if dirtying many inodes (which can delay umount) then
unmounting and quickly mounting again causes the mount to fail.

A race, which breaks the test in btrfs, is fixed by the patch:
"btrfs: fix mount failure caused by race with umount"

Signed-off-by: Boris Burkov <boris@bur.io>
---
- dd to XFS_IO_PROG
- 1M writes to 4k writes

 tests/generic/603     | 53 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/603.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 56 insertions(+)
 create mode 100755 tests/generic/603
 create mode 100644 tests/generic/603.out

diff --git a/tests/generic/603 b/tests/generic/603
new file mode 100755
index 00000000..90f0d1d3
--- /dev/null
+++ b/tests/generic/603
@@ -0,0 +1,53 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020 Facebook  All Rights Reserved.
+#
+# FS QA Test 603
+#
+# Evicting dirty inodes can take a long time during umount.
+# Check that a new mount racing with such a delayed umount succeeds.
+#
+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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+for i in $(seq 0 500)
+do
+	$XFS_IO_PROG -c "pwrite 0 4K" $SCRATCH_MNT/$i >/dev/null 2>&1
+done
+_scratch_unmount &
+_scratch_mount
+wait
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/603.out b/tests/generic/603.out
new file mode 100644
index 00000000..6810da89
--- /dev/null
+++ b/tests/generic/603.out
@@ -0,0 +1,2 @@
+QA output created by 603
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index d9ab9a31..c0ace35b 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -605,3 +605,4 @@
 600 auto quick quota
 601 auto quick quota
 602 auto quick encrypt
+603 auto quick
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4] generic: add a test for umount racing mount
  2020-07-20 19:05           ` [PATCH v4] " Boris Burkov
@ 2020-07-21  5:03             ` Zorro Lang
  0 siblings, 0 replies; 11+ messages in thread
From: Zorro Lang @ 2020-07-21  5:03 UTC (permalink / raw)
  To: fstests; +Cc: linux-btrfs

On Mon, Jul 20, 2020 at 12:05:56PM -0700, Boris Burkov wrote:
> Test if dirtying many inodes (which can delay umount) then
> unmounting and quickly mounting again causes the mount to fail.
> 
> A race, which breaks the test in btrfs, is fixed by the patch:
> "btrfs: fix mount failure caused by race with umount"
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---

This version looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

> - dd to XFS_IO_PROG
> - 1M writes to 4k writes
> 
>  tests/generic/603     | 53 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/603.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 56 insertions(+)
>  create mode 100755 tests/generic/603
>  create mode 100644 tests/generic/603.out
> 
> diff --git a/tests/generic/603 b/tests/generic/603
> new file mode 100755
> index 00000000..90f0d1d3
> --- /dev/null
> +++ b/tests/generic/603
> @@ -0,0 +1,53 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020 Facebook  All Rights Reserved.
> +#
> +# FS QA Test 603
> +#
> +# Evicting dirty inodes can take a long time during umount.
> +# Check that a new mount racing with such a delayed umount succeeds.
> +#
> +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.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +for i in $(seq 0 500)
> +do
> +	$XFS_IO_PROG -c "pwrite 0 4K" $SCRATCH_MNT/$i >/dev/null 2>&1
> +done
> +_scratch_unmount &
> +_scratch_mount
> +wait
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/603.out b/tests/generic/603.out
> new file mode 100644
> index 00000000..6810da89
> --- /dev/null
> +++ b/tests/generic/603.out
> @@ -0,0 +1,2 @@
> +QA output created by 603
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index d9ab9a31..c0ace35b 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -605,3 +605,4 @@
>  600 auto quick quota
>  601 auto quick quota
>  602 auto quick encrypt
> +603 auto quick
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-07-21  4:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10  0:55 [PATCH] btrfs: add a test for umount racing mount Boris Burkov
2020-07-10 14:35 ` Josef Bacik
2020-07-10 17:18   ` [PATCH v2] generic: " Boris Burkov
2020-07-10 17:52     ` Josef Bacik
2020-07-12 11:37     ` Zorro Lang
2020-07-13 20:46       ` [PATCH v3] " Boris Burkov
2020-07-14  5:21         ` Zorro Lang
2020-07-19 17:18         ` Eryu Guan
2020-07-20 19:05           ` [PATCH v4] " Boris Burkov
2020-07-21  5:03             ` Zorro Lang
2020-07-19 17:14     ` [PATCH v2] " Eryu Guan

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).