On Tue, Mar 01, 2022 at 03:19:30PM +0000, Sidong Yang wrote: > Test enabling/disable quota and creating/destroying qgroup repeatedly > in asynchronous and confirm it does not cause kernel hang. This is a in asynchronous -> in parallel > regression test for the problem reported to linux-btrfs list [1]. It's worth mentioning the deadlock only happens starting with kernel 5.17-rc3. > > The hang was recreated using the test case and fixed by kernel patch > titled > > btrfs: qgroup: fix deadlock between rescan worker and remove qgroup > > [1] https://lore.kernel.org/linux-btrfs/20220228014340.21309-1-realwakka@xxxxxxxxx/ > > Signed-off-by: Sidong Yang <realwakka@xxxxxxxxx> In addition to Shinichiro's comments... > --- > tests/btrfs/262 | 54 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/262.out | 2 ++ > 2 files changed, 56 insertions(+) > create mode 100755 tests/btrfs/262 > create mode 100644 tests/btrfs/262.out > > diff --git a/tests/btrfs/262 b/tests/btrfs/262 > new file mode 100755 > index 00000000..9be380f9 > --- /dev/null > +++ b/tests/btrfs/262 > @@ -0,0 +1,54 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 YOUR NAME HERE. All Rights Reserved. > +# > +# FS QA Test 262 > +# > +# Test the deadlock between qgroup and quota commands The test description should be a lot more clear. "the deadlock" is vague a gives the wrong idea we only ever had a single deadlock related to qgroups. "qgroup and quota commands" is confusing, and "qgroup" and "quota" are pretty much synonyms, and it should mention which commands. Plus what we want to test is that we can run some qgroup operations in parallel without triggering a deadlock, crash, etc. Perhaps something like: """ Test that running qgroup enable, create, destroy and disable commands in parallel does not result in a deadlock, a crash or any filesystem inconsistency. """ > +# > +. ./common/preamble > +_begin_fstest auto qgroup Can also be added to the "quick" group. It takes 1 second in my slowest vm. > + > +# Import common functions. > +. ./common/filter > + > +# real QA test starts here > + > +# Modify as appropriate. > +_supported_fs btrfs > + > +_require_scratch > + > +# Run command that enable/disable quota and create/destroy qgroup asynchronously With the more clear test description above, this can go away. > +qgroup_deadlock_test() > +{ > + _scratch_mkfs > /dev/null 2>&1 > + _scratch_mount > + echo "=== qgroup deadlock test ===" >> $seqres.full There's no point in echoing this message to the .full file, it provides no value at all, as testing that is all that this testcase does. > + > + pids=() > + for ((i = 0; i < 200; i++)); do > + $BTRFS_UTIL_PROG quota enable $SCRATCH_MNT 2>> $seqres.full & > + pids+=($!) > + $BTRFS_UTIL_PROG qgroup create 1/0 $SCRATCH_MNT 2>> $seqres.full & > + pids+=($!) > + $BTRFS_UTIL_PROG qgroup destroy 1/0 $SCRATCH_MNT 2>> $seqres.full & > + pids+=($!) > + $BTRFS_UTIL_PROG quota disable $SCRATCH_MNT 2>> $seqres.full & > + pids+=($!) > + done > + > + for pid in "${pids[@]}"; do > + wait $pid > + done As pointed before by Shinichiro, a simple 'wait' here is enough, no need to keep track of the PIDs. > + > + _scratch_unmount > + _check_scratch_fs Not needed, the fstests framework automatically runs 'btrfs check' when a test finishes. Doing this explicitly is only necessary when we need to do several mount/unmount operations and want to check the fs is fine after each unmount and before the next mount. > +} > + > +qgroup_deadlock_test There's no point in putting all the test code in a function, as the function is only called once. Otherwise it looks good, and the test works as advertised, it triggers a deadlock on 5.17-rc3+ kernel and passes on a patched kernel. Thanks for converting the reproducer into a test case. > + > +# success, all done > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/btrfs/262.out b/tests/btrfs/262.out > new file mode 100644 > index 00000000..404badc3 > --- /dev/null > +++ b/tests/btrfs/262.out > @@ -0,0 +1,2 @@ > +QA output created by 262 > +Silence is golden > -- > 2.25.1 >
