| Message ID | 20190930083735.21284-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: btrfs: Add regression test to check if btrfs can handle high devid | expand |
On 9/30/19 4:37 PM, Qu Wenruo wrote: > Add a regression test to check if btrfs can handle high devid. > > The test will add and remove devices to a btrfs fs, so that the devid > will increase to uncommon but still valid values. > > The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: > tree-checker: Verify dev item"). > The fix is titled "btrfs: tree-checker: Fix wrong check on max devid". > > Signed-off-by: Qu Wenruo <wqu@suse.com> Suggested-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Anand Jain <anand.jain@oracle.com> one nit below. > --- > tests/btrfs/194 | 73 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/194.out | 2 ++ > tests/btrfs/group | 1 + > 3 files changed, 76 insertions(+) > create mode 100755 tests/btrfs/194 > create mode 100644 tests/btrfs/194.out > > diff --git a/tests/btrfs/194 b/tests/btrfs/194 > new file mode 100755 > index 00000000..7a52ed86 > --- /dev/null > +++ b/tests/btrfs/194 > @@ -0,0 +1,73 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test 194 > +# > +# Test if btrfs can handle large device ids. > +# > +# The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: > +# tree-checker: Verify dev item"). > +# The fix is titlted: "btrfs: tree-checker: Fix wrong check on max devid" > +# > +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 btrfs > +_supported_os Linux > +_require_scratch_dev_pool 2 > +_scratch_dev_pool_get 2 > + > +# The wrong check limit is based on node size, to reduce runtime, we only > +# support 4K page size system for now > +if [ $(get_page_size) != 4096 ]; then > + _notrun "This test need 4k page size" > +fi > + > +device_1=$(echo $SCRATCH_DEV_POOL | awk '{print $1}') > +device_2=$(echo $SCRATCH_DEV_POOL | awk '{print $2}') > + > +echo device_1=$device_1 device_2=$device_2 >> $seqres.full > + > +# Use 4K nodesize to reduce runtime > +_scratch_mkfs -n 4k >> $seqres.full This init part should be _scratch_mkfs on only one device right? Thanks, Anand > +_scratch_mount > + > +# Add and remove device in a loop, one loop will increase devid by 2. > +# for 4k nodesize, the wrong check will be triggered at devid 123. > +# here 64 is enough to trigger such regression > +for (( i = 0; i < 64; i++ )); do > + $BTRFS_UTIL_PROG device add -f $device_2 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device del $device_1 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device add -f $device_1 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device del $device_2 $SCRATCH_MNT > +done > +_scratch_dev_pool_put > + > +echo "Silence is golden" > + > +# success, all done > +status=0 > +exit > diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out > new file mode 100644 > index 00000000..7bfd50ff > --- /dev/null > +++ b/tests/btrfs/194.out > @@ -0,0 +1,2 @@ > +QA output created by 194 > +Silence is golden > diff --git a/tests/btrfs/group b/tests/btrfs/group > index b92cb12c..ef1f0e1b 100644 > --- a/tests/btrfs/group > +++ b/tests/btrfs/group > @@ -196,3 +196,4 @@ > 191 auto quick send dedupe > 192 auto replay snapshot stress > 193 auto quick qgroup enospc limit > +194 auto >
On Mon, Sep 30, 2019 at 9:39 AM Qu Wenruo <wqu@suse.com> wrote: > > Add a regression test to check if btrfs can handle high devid. > > The test will add and remove devices to a btrfs fs, so that the devid > will increase to uncommon but still valid values. > > The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: > tree-checker: Verify dev item"). > The fix is titled "btrfs: tree-checker: Fix wrong check on max devid". > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > tests/btrfs/194 | 73 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/194.out | 2 ++ > tests/btrfs/group | 1 + > 3 files changed, 76 insertions(+) > create mode 100755 tests/btrfs/194 > create mode 100644 tests/btrfs/194.out > > diff --git a/tests/btrfs/194 b/tests/btrfs/194 > new file mode 100755 > index 00000000..7a52ed86 > --- /dev/null > +++ b/tests/btrfs/194 > @@ -0,0 +1,73 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test 194 > +# > +# Test if btrfs can handle large device ids. > +# > +# The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: > +# tree-checker: Verify dev item"). > +# The fix is titlted: "btrfs: tree-checker: Fix wrong check on max devid" type, titlted -> titled > +# > +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 btrfs > +_supported_os Linux > +_require_scratch_dev_pool 2 > +_scratch_dev_pool_get 2 > + > +# The wrong check limit is based on node size, to reduce runtime, we only > +# support 4K page size system for now > +if [ $(get_page_size) != 4096 ]; then > + _notrun "This test need 4k page size" > +fi > + > +device_1=$(echo $SCRATCH_DEV_POOL | awk '{print $1}') > +device_2=$(echo $SCRATCH_DEV_POOL | awk '{print $2}') > + > +echo device_1=$device_1 device_2=$device_2 >> $seqres.full > + > +# Use 4K nodesize to reduce runtime How does the node size reduces runtime? It's obvious when one wants to create deeper/larger trees for some purpose, but this test doesn't populate the filesystem, it uses an empty filesystem. So that deserves an explanation of how the node size influences the test's running time. > +_scratch_mkfs -n 4k >> $seqres.full > +_scratch_mount > + > +# Add and remove device in a loop, one loop will increase devid by 2. " ... one loop will ..." -> "... each iteration will ..." > +# for 4k nodesize, the wrong check will be triggered at devid 123. Why 123? It's not clear to me why, the test case doesn't explain and neither does the btrfs patch that fixes the regression. If it's related to the value of the constants/functions BTRFS_MAX_DEVS and BTRFS_MAX_DEVS_SYS_CHUNK, it should be mentioned here. > +# here 64 is enough to trigger such regression > +for (( i = 0; i < 64; i++ )); do > + $BTRFS_UTIL_PROG device add -f $device_2 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device del $device_1 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device add -f $device_1 $SCRATCH_MNT > + $BTRFS_UTIL_PROG device del $device_2 $SCRATCH_MNT > +done > +_scratch_dev_pool_put > + > +echo "Silence is golden" > + > +# success, all done > +status=0 > +exit > diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out > new file mode 100644 > index 00000000..7bfd50ff > --- /dev/null > +++ b/tests/btrfs/194.out > @@ -0,0 +1,2 @@ > +QA output created by 194 > +Silence is golden > diff --git a/tests/btrfs/group b/tests/btrfs/group > index b92cb12c..ef1f0e1b 100644 > --- a/tests/btrfs/group > +++ b/tests/btrfs/group > @@ -196,3 +196,4 @@ > 191 auto quick send dedupe > 192 auto replay snapshot stress > 193 auto quick qgroup enospc limit > +194 auto Maybe volume group as well. Thanks Qu. > -- > 2.22.0 >
On 2019/9/30 下午6:33, Filipe Manana wrote: > On Mon, Sep 30, 2019 at 9:39 AM Qu Wenruo <wqu@suse.com> wrote: >> >> Add a regression test to check if btrfs can handle high devid. >> >> The test will add and remove devices to a btrfs fs, so that the devid >> will increase to uncommon but still valid values. >> >> The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: >> tree-checker: Verify dev item"). >> The fix is titled "btrfs: tree-checker: Fix wrong check on max devid". >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> tests/btrfs/194 | 73 +++++++++++++++++++++++++++++++++++++++++++++ >> tests/btrfs/194.out | 2 ++ >> tests/btrfs/group | 1 + >> 3 files changed, 76 insertions(+) >> create mode 100755 tests/btrfs/194 >> create mode 100644 tests/btrfs/194.out >> >> diff --git a/tests/btrfs/194 b/tests/btrfs/194 >> new file mode 100755 >> index 00000000..7a52ed86 >> --- /dev/null >> +++ b/tests/btrfs/194 >> @@ -0,0 +1,73 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved. >> +# >> +# FS QA Test 194 >> +# >> +# Test if btrfs can handle large device ids. >> +# >> +# The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: >> +# tree-checker: Verify dev item"). >> +# The fix is titlted: "btrfs: tree-checker: Fix wrong check on max devid" > > type, titlted -> titled > >> +# >> +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 btrfs >> +_supported_os Linux >> +_require_scratch_dev_pool 2 >> +_scratch_dev_pool_get 2 >> + >> +# The wrong check limit is based on node size, to reduce runtime, we only >> +# support 4K page size system for now >> +if [ $(get_page_size) != 4096 ]; then >> + _notrun "This test need 4k page size" >> +fi >> + >> +device_1=$(echo $SCRATCH_DEV_POOL | awk '{print $1}') >> +device_2=$(echo $SCRATCH_DEV_POOL | awk '{print $2}') >> + >> +echo device_1=$device_1 device_2=$device_2 >> $seqres.full >> + >> +# Use 4K nodesize to reduce runtime > > How does the node size reduces runtime? The old wrong check is based on how large a chunk item can be. The macro we use is BTRFS_MAX_DEVS() which takes fs_info->nodesize to calculate. The largest chunk item (or any item) is based on nodesize. Thus nodesize will affect the runtime. > It's obvious when one wants to create deeper/larger trees for some > purpose, but this test doesn't populate the filesystem, it uses an > empty filesystem. > So that deserves an explanation of how the node size influences the > test's running time. Indeed. > >> +_scratch_mkfs -n 4k >> $seqres.full >> +_scratch_mount >> + >> +# Add and remove device in a loop, one loop will increase devid by 2. > > " ... one loop will ..." -> "... each iteration will ..." > >> +# for 4k nodesize, the wrong check will be triggered at devid 123. > > Why 123? It's not clear to me why, the test case doesn't explain and > neither does the btrfs patch that fixes the regression. > If it's related to the value of the constants/functions > BTRFS_MAX_DEVS and BTRFS_MAX_DEVS_SYS_CHUNK, it should be mentioned > here. OK, I'll mention > >> +# here 64 is enough to trigger such regression >> +for (( i = 0; i < 64; i++ )); do >> + $BTRFS_UTIL_PROG device add -f $device_2 $SCRATCH_MNT >> + $BTRFS_UTIL_PROG device del $device_1 $SCRATCH_MNT >> + $BTRFS_UTIL_PROG device add -f $device_1 $SCRATCH_MNT >> + $BTRFS_UTIL_PROG device del $device_2 $SCRATCH_MNT >> +done >> +_scratch_dev_pool_put >> + >> +echo "Silence is golden" >> + >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out >> new file mode 100644 >> index 00000000..7bfd50ff >> --- /dev/null >> +++ b/tests/btrfs/194.out >> @@ -0,0 +1,2 @@ >> +QA output created by 194 >> +Silence is golden >> diff --git a/tests/btrfs/group b/tests/btrfs/group >> index b92cb12c..ef1f0e1b 100644 >> --- a/tests/btrfs/group >> +++ b/tests/btrfs/group >> @@ -196,3 +196,4 @@ >> 191 auto quick send dedupe >> 192 auto replay snapshot stress >> 193 auto quick qgroup enospc limit >> +194 auto > > Maybe volume group as well. Thanks for the hint, I was looking into the groups but can't find a good candidate. At least volume makes more sense. Thanks, Qu > > Thanks Qu. > >> -- >> 2.22.0 >> > >
diff --git a/tests/btrfs/194 b/tests/btrfs/194 new file mode 100755 index 00000000..7a52ed86 --- /dev/null +++ b/tests/btrfs/194 @@ -0,0 +1,73 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2019 SUSE Linux Products GmbH. All Rights Reserved. +# +# FS QA Test 194 +# +# Test if btrfs can handle large device ids. +# +# The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: +# tree-checker: Verify dev item"). +# The fix is titlted: "btrfs: tree-checker: Fix wrong check on max devid" +# +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 btrfs +_supported_os Linux +_require_scratch_dev_pool 2 +_scratch_dev_pool_get 2 + +# The wrong check limit is based on node size, to reduce runtime, we only +# support 4K page size system for now +if [ $(get_page_size) != 4096 ]; then + _notrun "This test need 4k page size" +fi + +device_1=$(echo $SCRATCH_DEV_POOL | awk '{print $1}') +device_2=$(echo $SCRATCH_DEV_POOL | awk '{print $2}') + +echo device_1=$device_1 device_2=$device_2 >> $seqres.full + +# Use 4K nodesize to reduce runtime +_scratch_mkfs -n 4k >> $seqres.full +_scratch_mount + +# Add and remove device in a loop, one loop will increase devid by 2. +# for 4k nodesize, the wrong check will be triggered at devid 123. +# here 64 is enough to trigger such regression +for (( i = 0; i < 64; i++ )); do + $BTRFS_UTIL_PROG device add -f $device_2 $SCRATCH_MNT + $BTRFS_UTIL_PROG device del $device_1 $SCRATCH_MNT + $BTRFS_UTIL_PROG device add -f $device_1 $SCRATCH_MNT + $BTRFS_UTIL_PROG device del $device_2 $SCRATCH_MNT +done +_scratch_dev_pool_put + +echo "Silence is golden" + +# success, all done +status=0 +exit diff --git a/tests/btrfs/194.out b/tests/btrfs/194.out new file mode 100644 index 00000000..7bfd50ff --- /dev/null +++ b/tests/btrfs/194.out @@ -0,0 +1,2 @@ +QA output created by 194 +Silence is golden diff --git a/tests/btrfs/group b/tests/btrfs/group index b92cb12c..ef1f0e1b 100644 --- a/tests/btrfs/group +++ b/tests/btrfs/group @@ -196,3 +196,4 @@ 191 auto quick send dedupe 192 auto replay snapshot stress 193 auto quick qgroup enospc limit +194 auto
Add a regression test to check if btrfs can handle high devid. The test will add and remove devices to a btrfs fs, so that the devid will increase to uncommon but still valid values. The regression is introduced by kernel commit ab4ba2e13346 ("btrfs: tree-checker: Verify dev item"). The fix is titled "btrfs: tree-checker: Fix wrong check on max devid". Signed-off-by: Qu Wenruo <wqu@suse.com> --- tests/btrfs/194 | 73 +++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/194.out | 2 ++ tests/btrfs/group | 1 + 3 files changed, 76 insertions(+) create mode 100755 tests/btrfs/194 create mode 100644 tests/btrfs/194.out