diff mbox series

[RFC,2/2] ext4/061: Regression test of jbd2 journal_task race against unmount

Message ID 75819a20d2337c154e360c60ec53b7e519ebb97a.1682179372.git.ritesh.list@gmail.com
State Not Applicable
Headers show
Series [RFC,1/2] ext4/060: Regression test against dioread_nolock mount option inconsistency | expand

Commit Message

Ritesh Harjani (IBM) April 22, 2023, 4:17 p.m. UTC
This test adds a testcase to catch race condition against
reading of journal_task and parallel unmount.
This patch in kernel fixes this [1]

    ext4_put_super()                cat /sys/fs/ext4/loop2/journal_task
            |                               ext4_attr_show();
    ext4_jbd2_journal_destroy();                    |
            |                                journal_task_show()
            |                                       |
            |                               task_pid_vnr(NULL);
    sbi->s_journal = NULL;

RIP: 0010:__task_pid_nr_ns+0x4d/0xe0
<...>
Call Trace:
 <TASK>
 ext4_attr_show+0x1bd/0x3e0
 sysfs_kf_seq_show+0x8e/0x110
 seq_read_iter+0x11b/0x4d0
 vfs_read+0x216/0x2e0
 ksys_read+0x69/0xf0
 do_syscall_64+0x3f/0x90
 entry_SYSCALL_64_after_hwframe+0x72/0xdc
[

[1]: https://lore.kernel.org/all/20200318061301.4320-1-riteshh@linux.ibm.com/
Commit: ext4: Unregister sysfs path before destroying jbd2 journal

Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 tests/ext4/061     | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/061.out |  2 ++
 2 files changed, 84 insertions(+)
 create mode 100755 tests/ext4/061
 create mode 100644 tests/ext4/061.out

--
2.39.2

Comments

Zorro Lang April 23, 2023, 4:20 p.m. UTC | #1
On Sat, Apr 22, 2023 at 09:47:34PM +0530, Ritesh Harjani (IBM) wrote:
> This test adds a testcase to catch race condition against
> reading of journal_task and parallel unmount.
> This patch in kernel fixes this [1]
> 
>     ext4_put_super()                cat /sys/fs/ext4/loop2/journal_task
>             |                               ext4_attr_show();
>     ext4_jbd2_journal_destroy();                    |
>             |                                journal_task_show()
>             |                                       |
>             |                               task_pid_vnr(NULL);
>     sbi->s_journal = NULL;
> 
> RIP: 0010:__task_pid_nr_ns+0x4d/0xe0
> <...>
> Call Trace:
>  <TASK>
>  ext4_attr_show+0x1bd/0x3e0
>  sysfs_kf_seq_show+0x8e/0x110
>  seq_read_iter+0x11b/0x4d0
>  vfs_read+0x216/0x2e0
>  ksys_read+0x69/0xf0
>  do_syscall_64+0x3f/0x90
>  entry_SYSCALL_64_after_hwframe+0x72/0xdc
> [
> 
> [1]: https://lore.kernel.org/all/20200318061301.4320-1-riteshh@linux.ibm.com/
> Commit: ext4: Unregister sysfs path before destroying jbd2 journal
> 
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
>  tests/ext4/061     | 82 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/061.out |  2 ++
>  2 files changed, 84 insertions(+)
>  create mode 100755 tests/ext4/061
>  create mode 100644 tests/ext4/061.out
> 
> diff --git a/tests/ext4/061 b/tests/ext4/061
> new file mode 100755
> index 00000000..88bf138a
> --- /dev/null
> +++ b/tests/ext4/061
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 IBM Corporation.  All Rights Reserved.
> +#
> +# FS QA Test 061
> +#
> +# Regression test for https://lore.kernel.org/all/20200318061301.4320-1-riteshh@linux.ibm.com/
> +# ext4: Unregister sysfs path before destroying jbd2 journal

The link isn't needed, if you points out the commit id and subject.

> +#
> +
> +. ./common/preamble
> +_begin_fstest auto quick
> +
> +pid_mloop=""
> +pids_jloop=""
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	{
> +		kill -SIGKILL $pid_mloop $pids_jloop
> +		wait $pid_mloop $pids_jloop
> +	} > /dev/null 2>&1

[ -n "$pids_jloop" ] && kill -9 $pids_jloop
[ -n "$pid_mloop" ] && kill -9 $pid_mloop
wait

> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs ext4
> +_fixed_by_kernel_commit 5e47868fb94b63c \
> +	"ext4: unregister sysfs path before destroying jbd2 journal"
> +_require_scratch
> +_require_fs_sysfs journal_task
> +
> +_scratch_mkfs_ext4 >> $seqres.full 2>&1
> +# mount filesystem
> +_scratch_mount >> $seqres.full 2>&1
> +scratch_dev=$(_short_dev $SCRATCH_DEV)

The $scratch_dev is only used in read_journal_task_loop() ...

> +
> +function mount_loop()
> +{
> +	while [ 1 ]; do
> +		_scratch_unmount >> $seqres.full 2>&1
> +		sleep 1;
> +		_scratch_mount >> $seqres.full 2>&1

Do you hope to fail the test directly if the mount fails? Or you hope
to use _try_scratch_mount ?

> +		sleep 1;
> +	done
> +}
> +
> +function read_journal_task_loop()
> +{
> +	while [ 1 ]; do
> +		cat /sys/fs/ext4/$scratch_dev/journal_task > /dev/null 2>&1

... so I think you can write this line as:

cat /sys/fs/ext4/$(_short_dev $SCRATCH_DEV)/journal_task ...


BTW, I'm wondering if you need to check the journal_task is supported?

> +		sleep 1;
> +	done
> +}
> +
> +mount_loop &
> +pid_mloop=$!
> +
> +for i in $(seq 1 100); do
> +	read_journal_task_loop &
> +	pid=$!
> +	pids_jloop="${pids_jloop} ${pid}"

pids_jloop="${pids_jloop} $!"

> +done
> +
> +sleep 20

20 * TIME_FACTOR ?

> +{
> +	kill -SIGKILL $pid_mloop $pids_jloop
> +	wait $pid_mloop $pids_jloop
> +} > /dev/null 2>&1

kill -9 $pid_mloop $pids_jloop
wait $pid_mloop $pids_jloop
unset pid_mloop pids_jloop

> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/ext4/061.out b/tests/ext4/061.out
> new file mode 100644
> index 00000000..273be9e0
> --- /dev/null
> +++ b/tests/ext4/061.out
> @@ -0,0 +1,2 @@
> +QA output created by 061
> +Silence is golden
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/tests/ext4/061 b/tests/ext4/061
new file mode 100755
index 00000000..88bf138a
--- /dev/null
+++ b/tests/ext4/061
@@ -0,0 +1,82 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 IBM Corporation.  All Rights Reserved.
+#
+# FS QA Test 061
+#
+# Regression test for https://lore.kernel.org/all/20200318061301.4320-1-riteshh@linux.ibm.com/
+# ext4: Unregister sysfs path before destroying jbd2 journal
+#
+
+. ./common/preamble
+_begin_fstest auto quick
+
+pid_mloop=""
+pids_jloop=""
+trap "_cleanup; exit \$status" 0 1 2 3 15
+# Override the default cleanup function.
+_cleanup()
+{
+	{
+		kill -SIGKILL $pid_mloop $pids_jloop
+		wait $pid_mloop $pids_jloop
+	} > /dev/null 2>&1
+	cd /
+	rm -r -f $tmp.*
+}
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs ext4
+_fixed_by_kernel_commit 5e47868fb94b63c \
+	"ext4: unregister sysfs path before destroying jbd2 journal"
+_require_scratch
+_require_fs_sysfs journal_task
+
+_scratch_mkfs_ext4 >> $seqres.full 2>&1
+# mount filesystem
+_scratch_mount >> $seqres.full 2>&1
+scratch_dev=$(_short_dev $SCRATCH_DEV)
+
+function mount_loop()
+{
+	while [ 1 ]; do
+		_scratch_unmount >> $seqres.full 2>&1
+		sleep 1;
+		_scratch_mount >> $seqres.full 2>&1
+		sleep 1;
+	done
+}
+
+function read_journal_task_loop()
+{
+	while [ 1 ]; do
+		cat /sys/fs/ext4/$scratch_dev/journal_task > /dev/null 2>&1
+		sleep 1;
+	done
+}
+
+mount_loop &
+pid_mloop=$!
+
+for i in $(seq 1 100); do
+	read_journal_task_loop &
+	pid=$!
+	pids_jloop="${pids_jloop} ${pid}"
+done
+
+sleep 20
+{
+	kill -SIGKILL $pid_mloop $pids_jloop
+	wait $pid_mloop $pids_jloop
+} > /dev/null 2>&1
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/061.out b/tests/ext4/061.out
new file mode 100644
index 00000000..273be9e0
--- /dev/null
+++ b/tests/ext4/061.out
@@ -0,0 +1,2 @@ 
+QA output created by 061
+Silence is golden