[PATCH] btrfs: add regression test for remount with thread_pool resized

[PATCH] btrfs: add regression test for remount with thread_pool resized

[Xing Gu]

Regression test for resizing 'thread_pool' when remount the fs.

Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
---
 tests/btrfs/055     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/055.out |  1 +
 tests/btrfs/group   |  1 +
 3 files changed, 57 insertions(+)
 create mode 100755 tests/btrfs/055
 create mode 100644 tests/btrfs/055.out

diff --git a/tests/btrfs/055 b/tests/btrfs/055
new file mode 100755
index 0000000..0a0dd34
--- /dev/null
+++ b/tests/btrfs/055
@@ -0,0 +1,55 @@
+#!/bin/bash
+# FS QA Test No. btrfs/055
+#
+# Regression test for resizing 'thread_pool' when remount the fs.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null 2>&1
+
+_scratch_mount "-o thread_pool=6"
+
+_scratch_mount "-o remount,thread_pool=10"
+
+status=0 ; exit
diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
new file mode 100644
index 0000000..2fdd8f4
--- /dev/null
+++ b/tests/btrfs/055.out
@@ -0,0 +1 @@
+QA output created by 055
diff --git a/tests/btrfs/group b/tests/btrfs/group
index b668485..2c10c5b 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -57,3 +57,4 @@
 052 auto quick
 053 auto quick
 054 auto quick
+055 auto quick
-- 
1.9.0

Re: [PATCH] btrfs: add regression test for remount with thread_pool resized

[Dave Chinner]

On Fri, May 30, 2014 at 04:52:51PM +0800, Xing Gu wrote:
> Regression test for resizing 'thread_pool' when remount the fs.

Ping for btrfs test reviewers - is this test useful at all?

> Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> ---
>  tests/btrfs/055     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/055.out |  1 +
>  tests/btrfs/group   |  1 +
>  3 files changed, 57 insertions(+)
>  create mode 100755 tests/btrfs/055
>  create mode 100644 tests/btrfs/055.out
> 
> diff --git a/tests/btrfs/055 b/tests/btrfs/055
> new file mode 100755
> index 0000000..0a0dd34
> --- /dev/null
> +++ b/tests/btrfs/055
> @@ -0,0 +1,55 @@
> +#!/bin/bash
> +# FS QA Test No. btrfs/055
> +#
> +# Regression test for resizing 'thread_pool' when remount the fs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +    rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1
> +
> +_scratch_mount "-o thread_pool=6"
> +
> +_scratch_mount "-o remount,thread_pool=10"
> +
> +status=0 ; exit
> diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
> new file mode 100644
> index 0000000..2fdd8f4
> --- /dev/null
> +++ b/tests/btrfs/055.out
> @@ -0,0 +1 @@
> +QA output created by 055
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index b668485..2c10c5b 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -57,3 +57,4 @@
>  052 auto quick
>  053 auto quick
>  054 auto quick
> +055 auto quick
> -- 
> 1.9.0
> 
> --
> 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
> 

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] btrfs: add regression test for remount with thread_pool resized

[gux.fnst]

Hi,

    Could anyone help to review this patch? Thanks!

Regards,
Xing Gu

On 05/30/2014 04:52 PM, Xing Gu wrote:
> Regression test for resizing 'thread_pool' when remount the fs.
> 
> Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> ---
>   tests/btrfs/055     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/055.out |  1 +
>   tests/btrfs/group   |  1 +
>   3 files changed, 57 insertions(+)
>   create mode 100755 tests/btrfs/055
>   create mode 100644 tests/btrfs/055.out
> 
> diff --git a/tests/btrfs/055 b/tests/btrfs/055
> new file mode 100755
> index 0000000..0a0dd34
> --- /dev/null
> +++ b/tests/btrfs/055
> @@ -0,0 +1,55 @@
> +#!/bin/bash
> +# FS QA Test No. btrfs/055
> +#
> +# Regression test for resizing 'thread_pool' when remount the fs.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +    rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1
> +
> +_scratch_mount "-o thread_pool=6"
> +
> +_scratch_mount "-o remount,thread_pool=10"
> +
> +status=0 ; exit
> diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
> new file mode 100644
> index 0000000..2fdd8f4
> --- /dev/null
> +++ b/tests/btrfs/055.out
> @@ -0,0 +1 @@
> +QA output created by 055
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index b668485..2c10c5b 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -57,3 +57,4 @@
>   052 auto quick
>   053 auto quick
>   054 auto quick
> +055 auto quick
> 

Re: [PATCH] btrfs: add regression test for remount with thread_pool resized

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH] btrfs: add regression test for remount with 
thread_pool resized
From: gux.fnst <gux.fnst@cn.fujitsu.com>
To: <fstests@vger.kernel.org>
Date: 2014年09月17日 18:02
> Hi,
>
>      Could anyone help to review this patch? Thanks!
>
> Regards,
> Xing Gu
>
> On 05/30/2014 04:52 PM, Xing Gu wrote:
>> Regression test for resizing 'thread_pool' when remount the fs.
>>
>> Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
>> ---
>>    tests/btrfs/055     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>    tests/btrfs/055.out |  1 +
>>    tests/btrfs/group   |  1 +
>>    3 files changed, 57 insertions(+)
>>    create mode 100755 tests/btrfs/055
>>    create mode 100644 tests/btrfs/055.out
>>
>> diff --git a/tests/btrfs/055 b/tests/btrfs/055
>> new file mode 100755
>> index 0000000..0a0dd34
>> --- /dev/null
>> +++ b/tests/btrfs/055
>> @@ -0,0 +1,55 @@
>> +#!/bin/bash
>> +# FS QA Test No. btrfs/055
>> +#
>> +# Regression test for resizing 'thread_pool' when remount the fs.
>> +#
IMHO, it would be better to add the commit number for the regression and 
the fix commit number
(Filipe Manana always does the great job).

For reference:
Regresssion commit: 08a9ff3264181986d1d692a4e6fce3669700c9f8 (All my fault)
Fix commit: 800ee2247f483b6d05ed47ef3bbc90b56451746c

Other looks good for me.

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Thanks,
Qu
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +
>> +_cleanup()
>> +{
>> +    rm -f $tmp.*
>> +}
>> +
>> +trap "_cleanup ; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_scratch
>> +
>> +_scratch_mkfs > /dev/null 2>&1
>> +
>> +_scratch_mount "-o thread_pool=6"
>> +
>> +_scratch_mount "-o remount,thread_pool=10"
>> +
>> +status=0 ; exit
>> diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
>> new file mode 100644
>> index 0000000..2fdd8f4
>> --- /dev/null
>> +++ b/tests/btrfs/055.out
>> @@ -0,0 +1 @@
>> +QA output created by 055
>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index b668485..2c10c5b 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -57,3 +57,4 @@
>>    052 auto quick
>>    053 auto quick
>>    054 auto quick
>> +055 auto quick
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: add regression test for remount with thread_pool resized

[Dave Chinner]

On Tue, Dec 23, 2014 at 03:07:32PM +0800, Qu Wenruo wrote:
> 
> -------- Original Message --------
> Subject: Re: [PATCH] btrfs: add regression test for remount with
> thread_pool resized
> From: gux.fnst <gux.fnst@cn.fujitsu.com>
> To: <fstests@vger.kernel.org>
> Date: 2014年09月17日 18:02
> >Hi,
> >
> >     Could anyone help to review this patch? Thanks!
> >
> >Regards,
> >Xing Gu
> >
> >On 05/30/2014 04:52 PM, Xing Gu wrote:
> >>Regression test for resizing 'thread_pool' when remount the fs.
> >>
> >>Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> >>---
> >>   tests/btrfs/055     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   tests/btrfs/055.out |  1 +
> >>   tests/btrfs/group   |  1 +
> >>   3 files changed, 57 insertions(+)
> >>   create mode 100755 tests/btrfs/055
> >>   create mode 100644 tests/btrfs/055.out
> >>
> >>diff --git a/tests/btrfs/055 b/tests/btrfs/055
> >>new file mode 100755
> >>index 0000000..0a0dd34
> >>--- /dev/null
> >>+++ b/tests/btrfs/055
> >>@@ -0,0 +1,55 @@
> >>+#!/bin/bash
> >>+# FS QA Test No. btrfs/055
> >>+#
> >>+# Regression test for resizing 'thread_pool' when remount the fs.
> >>+#
> IMHO, it would be better to add the commit number for the regression
> and the fix commit number
> (Filipe Manana always does the great job).
> 
> For reference:
> Regresssion commit: 08a9ff3264181986d1d692a4e6fce3669700c9f8 (All my fault)
> Fix commit: 800ee2247f483b6d05ed47ef3bbc90b56451746c

That belongs in the commit message, not the test description.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH v2] btrfs: add regression test for remount with thread_pool resized

[Xing Gu]

Regression test for a btrfs issue of resizing 'thread_pool'
when remount the fs.

Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
---
 tests/btrfs/017     | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/017.out |  1 +
 tests/btrfs/group   |  1 +
 3 files changed, 66 insertions(+)
 create mode 100755 tests/btrfs/017
 create mode 100644 tests/btrfs/017.out

diff --git a/tests/btrfs/017 b/tests/btrfs/017
new file mode 100755
index 0000000..81f5af6
--- /dev/null
+++ b/tests/btrfs/017
@@ -0,0 +1,64 @@
+#!/bin/bash
+# FS QA Test No. btrfs/017
+#
+# Regression test for a btrfs issue of resizing 'thread_pool' when
+# remount the fs.
+# This was fixed in the following linux kernel patch:
+#     800ee22 btrfs: fix crash in remount(thread_pool=) case
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null
+
+dmesg -c > /dev/null
+nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`
+_scratch_mount "-o thread_pool=6"
+_scratch_mount "-o remount,thread_pool=10"
+new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
+if [ $new_null -ne $nr_null ]; then
+	_fatal "kernel bug detected, check dmesg for more infomation."
+fi
+
+status=0
+exit
diff --git a/tests/btrfs/017.out b/tests/btrfs/017.out
new file mode 100644
index 0000000..90c4b69
--- /dev/null
+++ b/tests/btrfs/017.out
@@ -0,0 +1 @@
+QA output created by 017
diff --git a/tests/btrfs/group b/tests/btrfs/group
index abb2fe4..cd553a3 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -19,6 +19,7 @@
 014 auto balance
 015 auto quick snapshot
 016 auto quick send
+017 auto quick
 018 auto quick subvol
 019 auto quick send
 020 auto quick replace
-- 
1.9.3

Re: [PATCH v2] btrfs: add regression test for remount with thread_pool resized

[Eryu Guan]

On Tue, Dec 23, 2014 at 01:41:02PM +0800, Xing Gu wrote:
> Regression test for a btrfs issue of resizing 'thread_pool'
> when remount the fs.
> 
> Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> ---
>  tests/btrfs/017     | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/017.out |  1 +
>  tests/btrfs/group   |  1 +
>  3 files changed, 66 insertions(+)
>  create mode 100755 tests/btrfs/017
>  create mode 100644 tests/btrfs/017.out
> 
> diff --git a/tests/btrfs/017 b/tests/btrfs/017
> new file mode 100755
> index 0000000..81f5af6
> --- /dev/null
> +++ b/tests/btrfs/017
> @@ -0,0 +1,64 @@
> +#!/bin/bash
> +# FS QA Test No. btrfs/017
> +#
> +# Regression test for a btrfs issue of resizing 'thread_pool' when
> +# remount the fs.
> +# This was fixed in the following linux kernel patch:
> +#     800ee22 btrfs: fix crash in remount(thread_pool=) case
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +    rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null
> +
> +dmesg -c > /dev/null
> +nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`

dmesg -c cleared dmesg buffer and nr_null is zero, I think this is
duplicated work, either dmesg -c or check nr_null before remount.

Actually I'm thinking about checking dmesg log in check after each test,
which could detect kernel BUG, WARNING and NULL pointer dereference etc.
so test doesn't need to check dmesg itself. I'll send it out for review
soon.

> +_scratch_mount "-o thread_pool=6"
> +_scratch_mount "-o remount,thread_pool=10"
> +new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
> +if [ $new_null -ne $nr_null ]; then
> +	_fatal "kernel bug detected, check dmesg for more infomation."

_fail is better than _fatal, and don't forget to remove $seqres.full before
test if you use _fail, cause it dumps errors to $seqres.full

But simply echo the error message could fail the test too, I'm fine with
either way..

> +fi
> +
> +status=0
> +exit
> diff --git a/tests/btrfs/017.out b/tests/btrfs/017.out
> new file mode 100644
> index 0000000..90c4b69
> --- /dev/null
> +++ b/tests/btrfs/017.out
> @@ -0,0 +1 @@
> +QA output created by 017
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index abb2fe4..cd553a3 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -19,6 +19,7 @@
>  014 auto balance
>  015 auto quick snapshot
>  016 auto quick send
> +017 auto quick

There's a remount group now, you can add it to remount group too.

Others look good to me.

Thanks,
Eryu
>  018 auto quick subvol
>  019 auto quick send
>  020 auto quick replace
> -- 
> 1.9.3
> 
> --
> 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

Re: [PATCH v2] btrfs: add regression test for remount with thread_pool resized

[Dave Chinner]

On Tue, Dec 23, 2014 at 03:23:10PM +0800, Eryu Guan wrote:
> On Tue, Dec 23, 2014 at 01:41:02PM +0800, Xing Gu wrote:
> > Regression test for a btrfs issue of resizing 'thread_pool'
> > when remount the fs.
> > 
> > Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> > ---
> >  tests/btrfs/017     | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/017.out |  1 +
> >  tests/btrfs/group   |  1 +
> >  3 files changed, 66 insertions(+)
> >  create mode 100755 tests/btrfs/017
> >  create mode 100644 tests/btrfs/017.out
> > 
> > diff --git a/tests/btrfs/017 b/tests/btrfs/017
> > new file mode 100755
> > index 0000000..81f5af6
> > --- /dev/null
> > +++ b/tests/btrfs/017
> > @@ -0,0 +1,64 @@
> > +#!/bin/bash
> > +# FS QA Test No. btrfs/017
> > +#
> > +# Regression test for a btrfs issue of resizing 'thread_pool' when
> > +# remount the fs.
> > +# This was fixed in the following linux kernel patch:
> > +#     800ee22 btrfs: fix crash in remount(thread_pool=) case
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > +#
> > +#-----------------------------------------------------------------------
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +
> > +_cleanup()
> > +{
> > +    rm -f $tmp.*
> > +}
> > +
> > +trap "_cleanup ; exit \$status" 0 1 2 3 15
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs btrfs
> > +_supported_os Linux
> > +_require_scratch
> > +
> > +_scratch_mkfs > /dev/null
> > +
> > +dmesg -c > /dev/null
> > +nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`
> 
> dmesg -c cleared dmesg buffer and nr_null is zero, I think this is
> duplicated work, either dmesg -c or check nr_null before remount.

Please don't clear the dmesg buffer - it means that people can't
review the kernel error messages across the entire test run for
errors unless they are capturing that information elsewhere.

And yes, I don't use syslog for capturing dmesg output on my test
boxes because that leads to root filesystem ENOSPC whenever I'm
dumping a lot of debug to the console....

> Actually I'm thinking about checking dmesg log in check after each test,
> which could detect kernel BUG, WARNING and NULL pointer dereference etc.
> so test doesn't need to check dmesg itself. I'll send it out for review
> soon.

Some tests actually generate errors/warnings on purpose. IOWs, the
presence of messages in dmesg is not always indicative of an
error....

> > +_scratch_mount "-o remount,thread_pool=10"
> > +new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
> > +if [ $new_null -ne $nr_null ]; then
> > +	_fatal "kernel bug detected, check dmesg for more infomation."
> 
> _fail is better than _fatal, and don't forget to remove $seqres.full before
> test if you use _fail, cause it dumps errors to $seqres.full
> 
> But simply echo the error message could fail the test too, I'm fine with
> either way..

Echo the error message, set status=1 and exit.

Oh, wait, that's what _fail does. ;)

> > +exit
> > diff --git a/tests/btrfs/017.out b/tests/btrfs/017.out
> > new file mode 100644
> > index 0000000..90c4b69
> > --- /dev/null
> > +++ b/tests/btrfs/017.out
> > @@ -0,0 +1 @@
> > +QA output created by 017

Empty output with no indication that it should be empty is bad form
as well. The test should echo something to indicate that an empty
output file is expected. "Silence is golden" is the usual string
used to indicate that the golden output expects no output from the
test.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v2] btrfs: add regression test for remount with thread_pool resized

[Eryu Guan]

On Wed, Dec 24, 2014 at 07:56:07AM +1100, Dave Chinner wrote:
> On Tue, Dec 23, 2014 at 03:23:10PM +0800, Eryu Guan wrote:
> > On Tue, Dec 23, 2014 at 01:41:02PM +0800, Xing Gu wrote:
> > > Regression test for a btrfs issue of resizing 'thread_pool'
> > > when remount the fs.
> > > 
> > > Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
> > > ---
> > >  tests/btrfs/017     | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/017.out |  1 +
> > >  tests/btrfs/group   |  1 +
> > >  3 files changed, 66 insertions(+)
> > >  create mode 100755 tests/btrfs/017
> > >  create mode 100644 tests/btrfs/017.out
> > > 
> > > diff --git a/tests/btrfs/017 b/tests/btrfs/017
> > > new file mode 100755
> > > index 0000000..81f5af6
> > > --- /dev/null
> > > +++ b/tests/btrfs/017
> > > @@ -0,0 +1,64 @@
> > > +#!/bin/bash
> > > +# FS QA Test No. btrfs/017
> > > +#
> > > +# Regression test for a btrfs issue of resizing 'thread_pool' when
> > > +# remount the fs.
> > > +# This was fixed in the following linux kernel patch:
> > > +#     800ee22 btrfs: fix crash in remount(thread_pool=) case
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
> > > +#
> > > +# This program is free software; you can redistribute it and/or
> > > +# modify it under the terms of the GNU General Public License as
> > > +# published by the Free Software Foundation.
> > > +#
> > > +# This program is distributed in the hope that it would be useful,
> > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +# GNU General Public License for more details.
> > > +#
> > > +# You should have received a copy of the GNU General Public License
> > > +# along with this program; if not, write the Free Software Foundation,
> > > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > > +#
> > > +#-----------------------------------------------------------------------
> > > +#
> > > +
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> > > +here=`pwd`
> > > +tmp=/tmp/$$
> > > +status=1	# failure is the default!
> > > +
> > > +_cleanup()
> > > +{
> > > +    rm -f $tmp.*
> > > +}
> > > +
> > > +trap "_cleanup ; exit \$status" 0 1 2 3 15
> > > +
> > > +# get standard environment, filters and checks
> > > +. ./common/rc
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs btrfs
> > > +_supported_os Linux
> > > +_require_scratch
> > > +
> > > +_scratch_mkfs > /dev/null
> > > +
> > > +dmesg -c > /dev/null
> > > +nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`
> > 
> > dmesg -c cleared dmesg buffer and nr_null is zero, I think this is
> > duplicated work, either dmesg -c or check nr_null before remount.
> 
> Please don't clear the dmesg buffer - it means that people can't
> review the kernel error messages across the entire test run for
> errors unless they are capturing that information elsewhere.
> 
> And yes, I don't use syslog for capturing dmesg output on my test
> boxes because that leads to root filesystem ENOSPC whenever I'm
> dumping a lot of debug to the console....
> 
> > Actually I'm thinking about checking dmesg log in check after each test,
> > which could detect kernel BUG, WARNING and NULL pointer dereference etc.
> > so test doesn't need to check dmesg itself. I'll send it out for review
> > soon.
> 
> Some tests actually generate errors/warnings on purpose. IOWs, the
> presence of messages in dmesg is not always indicative of an
> error....

Yes, not all tests should be checked, something like
_require_scratch_nocheck may help here, and add that function call to all
tests that generate errors/warnings on purpose.

I'll think about it more, to see how to avoid clearing dmesg buffer also
avoid dumping all the dmesg log to some file, if all the checking dmesg
log thing is reasonable.

Thanks,
Eryu
> 
> > > +_scratch_mount "-o remount,thread_pool=10"
> > > +new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
> > > +if [ $new_null -ne $nr_null ]; then
> > > +	_fatal "kernel bug detected, check dmesg for more infomation."
> > 
> > _fail is better than _fatal, and don't forget to remove $seqres.full before
> > test if you use _fail, cause it dumps errors to $seqres.full
> > 
> > But simply echo the error message could fail the test too, I'm fine with
> > either way..
> 
> Echo the error message, set status=1 and exit.
> 
> Oh, wait, that's what _fail does. ;)
> 
> > > +exit
> > > diff --git a/tests/btrfs/017.out b/tests/btrfs/017.out
> > > new file mode 100644
> > > index 0000000..90c4b69
> > > --- /dev/null
> > > +++ b/tests/btrfs/017.out
> > > @@ -0,0 +1 @@
> > > +QA output created by 017
> 
> Empty output with no indication that it should be empty is bad form
> as well. The test should echo something to indicate that an empty
> output file is expected. "Silence is golden" is the usual string
> used to indicate that the golden output expects no output from the
> test.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> 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

[PATCH v3] btrfs: add regression test for remount with thread_pool resized

[Xing Gu]

Regression test for a btrfs issue of resizing 'thread_pool' when
remount the fs.

This regression was introduced by the following linux kernel commit:
    btrfs: Added btrfs_workqueue_struct implemented ordered
    execution based on kernel workqueue
    08a9ff3264181986d1d692a4e6fce3669700c9f8
And it was fixed by the following linux kernel commit:
    btrfs: fix crash in remount(thread_pool=) case
    800ee2247f483b6d05ed47ef3bbc90b56451746c

Signed-off-by: Xing Gu <gux.fnst@cn.fujitsu.com>
---
 tests/btrfs/082     | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/082.out |  2 ++
 tests/btrfs/group   |  1 +
 3 files changed, 73 insertions(+)
 create mode 100755 tests/btrfs/082
 create mode 100644 tests/btrfs/082.out

diff --git a/tests/btrfs/082 b/tests/btrfs/082
new file mode 100755
index 0000000..987ffae
--- /dev/null
+++ b/tests/btrfs/082
@@ -0,0 +1,70 @@
+#!/bin/bash
+# FS QA Test No. btrfs/082
+#
+# Regression test for a btrfs issue of resizing 'thread_pool' when
+# remount the fs.
+#
+# This regression was introduced by the following linux kernel commit:
+#     btrfs: Added btrfs_workqueue_struct implemented ordered
+#     execution based on kernel workqueue
+#     08a9ff3264181986d1d692a4e6fce3669700c9f8
+# And it was fixed by the following linux kernel commit:
+#     btrfs: fix crash in remount(thread_pool=) case
+#     800ee2247f483b6d05ed47ef3bbc90b56451746c
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null
+
+nr_null=`dmesg | grep -c "kernel NULL pointer dereference"`
+_scratch_mount "-o thread_pool=6"
+_scratch_mount "-o remount,thread_pool=10"
+new_null=`dmesg | grep -c "kernel NULL pointer dereference"`
+if [ $new_null -ne $nr_null ]; then
+	_fail "kernel bug detected, check dmesg for more infomation."
+fi
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/082.out b/tests/btrfs/082.out
new file mode 100644
index 0000000..2977f14
--- /dev/null
+++ b/tests/btrfs/082.out
@@ -0,0 +1,2 @@
+QA output created by 082
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index e29999b..fd2fa76 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -84,3 +84,4 @@
 079 auto rw metadata
 080 auto snapshot
 081 auto quick clone
+082 auto quick remount
-- 
1.9.3