[PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Lu Fengqi]

Test if qgroup can handle extent de-reference during reallocation.
"extent de-reference" means that reducing an extent's reference count
or freeing an extent.
Although current qgroup can handle it, we still need to prevent any
regression which may break current qgroup.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 common/rc           |  4 +--
 tests/btrfs/028     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/028.out |  2 ++
 tests/btrfs/group   |  1 +
 4 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100755 tests/btrfs/028
 create mode 100644 tests/btrfs/028.out

diff --git a/common/rc b/common/rc
index 51092a0..650d198 100644
--- a/common/rc
+++ b/common/rc
@@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
 # stress btrfs by running balance operation in a loop
 _btrfs_stress_balance()
 {
-	local btrfs_mnt=$1
+	local options=$@
 	while true; do
-		$BTRFS_UTIL_PROG balance start $btrfs_mnt
+		$BTRFS_UTIL_PROG balance start $options
 	done
 }
 
diff --git a/tests/btrfs/028 b/tests/btrfs/028
new file mode 100755
index 0000000..8cea49a
--- /dev/null
+++ b/tests/btrfs/028
@@ -0,0 +1,98 @@
+#! /bin/bash
+# FS QA Test 028
+#
+# Test if qgroup can handle extent de-reference during reallocation.
+# "extent de-reference" means that reducing an extent's reference count
+# or freeing an extent.
+# Although current qgroup can handle it, we still need to prevent any
+# regression which may break current qgroup.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2016 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!
+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
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs
+_scratch_mount
+
+_run_btrfs_util_prog quota enable $SCRATCH_MNT
+_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
+
+# Increase the probability of generating de-refer extent, and decrease
+# other.
+args=`_scale_fsstress_args -z \
+	-f write=10 -f unlink=10 \
+	-f creat=10 -f fsync=10 \
+	-f fsync=10 -n 100000 -p 2 \
+	-d $SCRATCH_MNT/stress_dir`
+echo "Run fsstress $args" >>$seqres.full
+$FSSTRESS_PROG $args >/dev/null 2>&1 &
+fsstress_pid=$!
+
+echo "Start balance" >>$seqres.full
+_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
+balance_pid=$!
+
+# 30s is enough to trigger bug
+sleep $((30*$TIME_FACTOR))
+kill $fsstress_pid $balance_pid
+wait
+
+# kill _btrfs_stress_balance can't end balance, so call btrfs balance cancel
+# to cancel running or paused balance.
+$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
+
+_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
+
+_scratch_unmount
+
+# generate a qgroup report and look for inconsistent groups
+$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
+			grep -q -E "Counts for qgroup.*are different"
+if [ $? -ne 0 ]; then
+	echo "Silence is golden"
+	# success, all done
+	status=0
+fi
+
+exit
diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
new file mode 100644
index 0000000..2615f73
--- /dev/null
+++ b/tests/btrfs/028.out
@@ -0,0 +1,2 @@
+QA output created by 028
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index da0e27f..35ecf59 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -30,6 +30,7 @@
 025 auto quick send clone
 026 auto quick compress prealloc
 027 auto replace
+028 auto qgroup balance
 029 auto quick clone
 030 auto quick send
 031 auto quick subvol clone
-- 
2.5.5

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Eryu Guan]

On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
> Test if qgroup can handle extent de-reference during reallocation.
> "extent de-reference" means that reducing an extent's reference count
> or freeing an extent.
> Although current qgroup can handle it, we still need to prevent any
> regression which may break current qgroup.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  common/rc           |  4 +--
>  tests/btrfs/028     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/028.out |  2 ++
>  tests/btrfs/group   |  1 +
>  4 files changed, 103 insertions(+), 2 deletions(-)
>  create mode 100755 tests/btrfs/028
>  create mode 100644 tests/btrfs/028.out
> 
> diff --git a/common/rc b/common/rc
> index 51092a0..650d198 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>  # stress btrfs by running balance operation in a loop
>  _btrfs_stress_balance()
>  {
> -	local btrfs_mnt=$1
> +	local options=$@
>  	while true; do
> -		$BTRFS_UTIL_PROG balance start $btrfs_mnt
> +		$BTRFS_UTIL_PROG balance start $options
>  	done
>  }
>  
> diff --git a/tests/btrfs/028 b/tests/btrfs/028
> new file mode 100755
> index 0000000..8cea49a
> --- /dev/null
> +++ b/tests/btrfs/028
> @@ -0,0 +1,98 @@
> +#! /bin/bash
> +# FS QA Test 028
> +#
> +# Test if qgroup can handle extent de-reference during reallocation.
> +# "extent de-reference" means that reducing an extent's reference count
> +# or freeing an extent.
> +# Although current qgroup can handle it, we still need to prevent any
> +# regression which may break current qgroup.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2016 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!
> +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
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs
> +_scratch_mount
> +
> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
> +
> +# Increase the probability of generating de-refer extent, and decrease
> +# other.
> +args=`_scale_fsstress_args -z \
> +	-f write=10 -f unlink=10 \
> +	-f creat=10 -f fsync=10 \
> +	-f fsync=10 -n 100000 -p 2 \
> +	-d $SCRATCH_MNT/stress_dir`
> +echo "Run fsstress $args" >>$seqres.full
> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
> +fsstress_pid=$!
> +
> +echo "Start balance" >>$seqres.full
> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
> +balance_pid=$!
> +
> +# 30s is enough to trigger bug
> +sleep $((30*$TIME_FACTOR))
> +kill $fsstress_pid $balance_pid
> +wait
> +
> +# kill _btrfs_stress_balance can't end balance, so call btrfs balance cancel
> +# to cancel running or paused balance.
> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
> +
> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
> +
> +_scratch_unmount
> +
> +# generate a qgroup report and look for inconsistent groups
> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
> +			grep -q -E "Counts for qgroup.*are different"
> +if [ $? -ne 0 ]; then
> +	echo "Silence is golden"
> +	# success, all done
> +	status=0
> +fi

I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
which means btrfs check finds inconsistent groups. But according to your
commit log, current kernel should pass the test. So is the failure
expected?

Also, just grep for different qgroup counts and print the message out if
grep finds the message, so it breaks golden image on error and we know
something really goes wrong. Right now test fails just because of
missing "Silence is golden", which is unclear why it fails:

 @@ -1,2 +1 @@
  QA output created by 028
 -Silence is golden

Do the following instead:

	$BTRFS_UTIL_PROG check ... | grep -E "..."
	echo "Silence is golden"
	status=0
	exit

And we see this on failure:

 @@ -1,2 +1,3 @@
  QA output created by 028
 +Counts for qgroup id: 5 are different
  Silence is golden

Thanks,
Eryu

> +
> +exit
> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
> new file mode 100644
> index 0000000..2615f73
> --- /dev/null
> +++ b/tests/btrfs/028.out
> @@ -0,0 +1,2 @@
> +QA output created by 028
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index da0e27f..35ecf59 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -30,6 +30,7 @@
>  025 auto quick send clone
>  026 auto quick compress prealloc
>  027 auto replace
> +028 auto qgroup balance
>  029 auto quick clone
>  030 auto quick send
>  031 auto quick subvol clone
> -- 
> 2.5.5
> 
> 
> 
> --
> 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 v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Lu Fengqi]

At 06/13/2016 11:04 AM, Eryu Guan wrote:
> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>> Test if qgroup can handle extent de-reference during reallocation.
>> "extent de-reference" means that reducing an extent's reference count
>> or freeing an extent.
>> Although current qgroup can handle it, we still need to prevent any
>> regression which may break current qgroup.
>>
>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> ---
>>  common/rc           |  4 +--
>>  tests/btrfs/028     | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/028.out |  2 ++
>>  tests/btrfs/group   |  1 +
>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>  create mode 100755 tests/btrfs/028
>>  create mode 100644 tests/btrfs/028.out
>>
>> diff --git a/common/rc b/common/rc
>> index 51092a0..650d198 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>  # stress btrfs by running balance operation in a loop
>>  _btrfs_stress_balance()
>>  {
>> -	local btrfs_mnt=$1
>> +	local options=$@
>>  	while true; do
>> -		$BTRFS_UTIL_PROG balance start $btrfs_mnt
>> +		$BTRFS_UTIL_PROG balance start $options
>>  	done
>>  }
>>
>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>> new file mode 100755
>> index 0000000..8cea49a
>> --- /dev/null
>> +++ b/tests/btrfs/028
>> @@ -0,0 +1,98 @@
>> +#! /bin/bash
>> +# FS QA Test 028
>> +#
>> +# Test if qgroup can handle extent de-reference during reallocation.
>> +# "extent de-reference" means that reducing an extent's reference count
>> +# or freeing an extent.
>> +# Although current qgroup can handle it, we still need to prevent any
>> +# regression which may break current qgroup.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2016 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!
>> +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
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_scratch
>> +
>> +_scratch_mkfs
>> +_scratch_mount
>> +
>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>> +
>> +# Increase the probability of generating de-refer extent, and decrease
>> +# other.
>> +args=`_scale_fsstress_args -z \
>> +	-f write=10 -f unlink=10 \
>> +	-f creat=10 -f fsync=10 \
>> +	-f fsync=10 -n 100000 -p 2 \
>> +	-d $SCRATCH_MNT/stress_dir`
>> +echo "Run fsstress $args" >>$seqres.full
>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>> +fsstress_pid=$!
>> +
>> +echo "Start balance" >>$seqres.full
>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>> +balance_pid=$!
>> +
>> +# 30s is enough to trigger bug
>> +sleep $((30*$TIME_FACTOR))
>> +kill $fsstress_pid $balance_pid
>> +wait
>> +
>> +# kill _btrfs_stress_balance can't end balance, so call btrfs balance cancel
>> +# to cancel running or paused balance.
>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>> +
>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>> +
>> +_scratch_unmount
>> +
>> +# generate a qgroup report and look for inconsistent groups
>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>> +			grep -q -E "Counts for qgroup.*are different"
>> +if [ $? -ne 0 ]; then
>> +	echo "Silence is golden"
>> +	# success, all done
>> +	status=0
>> +fi
>
> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
> which means btrfs check finds inconsistent groups. But according to your
> commit log, current kernel should pass the test. So is the failure
> expected?
>
> Also, just grep for different qgroup counts and print the message out if
> grep finds the message, so it breaks golden image on error and we know
> something really goes wrong. Right now test fails just because of
> missing "Silence is golden", which is unclear why it fails:
>
>  @@ -1,2 +1 @@
>   QA output created by 028
>  -Silence is golden
>
> Do the following instead:
>
> 	$BTRFS_UTIL_PROG check ... | grep -E "..."
> 	echo "Silence is golden"
> 	status=0
> 	exit
>
> And we see this on failure:
>
>  @@ -1,2 +1,3 @@
>   QA output created by 028
>  +Counts for qgroup id: 5 are different
>   Silence is golden
>
> Thanks,
> Eryu
>
>> +
>> +exit
>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>> new file mode 100644
>> index 0000000..2615f73
>> --- /dev/null
>> +++ b/tests/btrfs/028.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 028
>> +Silence is golden
>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index da0e27f..35ecf59 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -30,6 +30,7 @@
>>  025 auto quick send clone
>>  026 auto quick compress prealloc
>>  027 auto replace
>> +028 auto qgroup balance
>>  029 auto quick clone
>>  030 auto quick send
>>  031 auto quick subvol clone
>> --
>> 2.5.5
>>
>>
>>
>> --
>> 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
>
>

Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 / 
4.7-rc3), the test didn't fail. Can you provide me with more information 
about your test environment?


-- 
Thanks,
Lu

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Lu Fengqi]

At 06/13/2016 03:29 PM, Lu Fengqi wrote:
> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>> Test if qgroup can handle extent de-reference during reallocation.
>>> "extent de-reference" means that reducing an extent's reference count
>>> or freeing an extent.
>>> Although current qgroup can handle it, we still need to prevent any
>>> regression which may break current qgroup.
>>>
>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>> ---
>>>  common/rc           |  4 +--
>>>  tests/btrfs/028     | 98
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/btrfs/028.out |  2 ++
>>>  tests/btrfs/group   |  1 +
>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>  create mode 100755 tests/btrfs/028
>>>  create mode 100644 tests/btrfs/028.out
>>>
>>> diff --git a/common/rc b/common/rc
>>> index 51092a0..650d198 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>  # stress btrfs by running balance operation in a loop
>>>  _btrfs_stress_balance()
>>>  {
>>> -    local btrfs_mnt=$1
>>> +    local options=$@
>>>      while true; do
>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>> +        $BTRFS_UTIL_PROG balance start $options
>>>      done
>>>  }
>>>
>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>> new file mode 100755
>>> index 0000000..8cea49a
>>> --- /dev/null
>>> +++ b/tests/btrfs/028
>>> @@ -0,0 +1,98 @@
>>> +#! /bin/bash
>>> +# FS QA Test 028
>>> +#
>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>> +# "extent de-reference" means that reducing an extent's reference count
>>> +# or freeing an extent.
>>> +# Although current qgroup can handle it, we still need to prevent any
>>> +# regression which may break current qgroup.
>>> +#
>>> +#-----------------------------------------------------------------------
>>>
>>> +# Copyright (c) 2016 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!
>>> +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
>>> +_supported_fs btrfs
>>> +_supported_os Linux
>>> +_require_scratch
>>> +
>>> +_scratch_mkfs
>>> +_scratch_mount
>>> +
>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>> +
>>> +# Increase the probability of generating de-refer extent, and decrease
>>> +# other.
>>> +args=`_scale_fsstress_args -z \
>>> +    -f write=10 -f unlink=10 \
>>> +    -f creat=10 -f fsync=10 \
>>> +    -f fsync=10 -n 100000 -p 2 \
>>> +    -d $SCRATCH_MNT/stress_dir`
>>> +echo "Run fsstress $args" >>$seqres.full
>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>> +fsstress_pid=$!
>>> +
>>> +echo "Start balance" >>$seqres.full
>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>> +balance_pid=$!
>>> +
>>> +# 30s is enough to trigger bug
>>> +sleep $((30*$TIME_FACTOR))
>>> +kill $fsstress_pid $balance_pid
>>> +wait
>>> +
>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>> balance cancel
>>> +# to cancel running or paused balance.
>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>> +
>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>> +
>>> +_scratch_unmount
>>> +
>>> +# generate a qgroup report and look for inconsistent groups
>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>> +            grep -q -E "Counts for qgroup.*are different"
>>> +if [ $? -ne 0 ]; then
>>> +    echo "Silence is golden"
>>> +    # success, all done
>>> +    status=0
>>> +fi
>>
>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>> which means btrfs check finds inconsistent groups. But according to your
>> commit log, current kernel should pass the test. So is the failure
>> expected?
>>
>> Also, just grep for different qgroup counts and print the message out if
>> grep finds the message, so it breaks golden image on error and we know
>> something really goes wrong. Right now test fails just because of
>> missing "Silence is golden", which is unclear why it fails:
>>
>>  @@ -1,2 +1 @@
>>   QA output created by 028
>>  -Silence is golden
>>
>> Do the following instead:
>>
>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>     echo "Silence is golden"
>>     status=0
>>     exit
>>
>> And we see this on failure:
>>
>>  @@ -1,2 +1,3 @@
>>   QA output created by 028
>>  +Counts for qgroup id: 5 are different
>>   Silence is golden
>>
>> Thanks,
>> Eryu
>>
>>> +
>>> +exit
>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>> new file mode 100644
>>> index 0000000..2615f73
>>> --- /dev/null
>>> +++ b/tests/btrfs/028.out
>>> @@ -0,0 +1,2 @@
>>> +QA output created by 028
>>> +Silence is golden
>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>> index da0e27f..35ecf59 100644
>>> --- a/tests/btrfs/group
>>> +++ b/tests/btrfs/group
>>> @@ -30,6 +30,7 @@
>>>  025 auto quick send clone
>>>  026 auto quick compress prealloc
>>>  027 auto replace
>>> +028 auto qgroup balance
>>>  029 auto quick clone
>>>  030 auto quick send
>>>  031 auto quick subvol clone
>>> --
>>> 2.5.5
>>>
>>>
>>>
>>> --
>>> 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
>>
>>
>
> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
> 4.7-rc3), the test didn't fail. Can you provide me with more information
> about your test environment?
>
>
Sorry, this test fails after I perform it more times. It seems to be a 
bug of btrfs qgroup. We will continue to track this bug.

-- 
Thanks,
Lu

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Filipe Manana]

On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com> wrote:
> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>
>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>
>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>
>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>> "extent de-reference" means that reducing an extent's reference count
>>>> or freeing an extent.
>>>> Although current qgroup can handle it, we still need to prevent any
>>>> regression which may break current qgroup.
>>>>
>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>> ---
>>>>  common/rc           |  4 +--
>>>>  tests/btrfs/028     | 98
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  tests/btrfs/028.out |  2 ++
>>>>  tests/btrfs/group   |  1 +
>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>  create mode 100755 tests/btrfs/028
>>>>  create mode 100644 tests/btrfs/028.out
>>>>
>>>> diff --git a/common/rc b/common/rc
>>>> index 51092a0..650d198 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>  # stress btrfs by running balance operation in a loop
>>>>  _btrfs_stress_balance()
>>>>  {
>>>> -    local btrfs_mnt=$1
>>>> +    local options=$@
>>>>      while true; do
>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>      done
>>>>  }
>>>>
>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>> new file mode 100755
>>>> index 0000000..8cea49a
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/028
>>>> @@ -0,0 +1,98 @@
>>>> +#! /bin/bash
>>>> +# FS QA Test 028
>>>> +#
>>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>>> +# "extent de-reference" means that reducing an extent's reference count
>>>> +# or freeing an extent.
>>>> +# Although current qgroup can handle it, we still need to prevent any
>>>> +# regression which may break current qgroup.
>>>> +#
>>>>
>>>> +#-----------------------------------------------------------------------
>>>>
>>>> +# Copyright (c) 2016 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!
>>>> +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
>>>> +_supported_fs btrfs
>>>> +_supported_os Linux
>>>> +_require_scratch
>>>> +
>>>> +_scratch_mkfs
>>>> +_scratch_mount
>>>> +
>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>> +
>>>> +# Increase the probability of generating de-refer extent, and decrease
>>>> +# other.
>>>> +args=`_scale_fsstress_args -z \
>>>> +    -f write=10 -f unlink=10 \
>>>> +    -f creat=10 -f fsync=10 \
>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>> +echo "Run fsstress $args" >>$seqres.full
>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>> +fsstress_pid=$!
>>>> +
>>>> +echo "Start balance" >>$seqres.full
>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>> +balance_pid=$!
>>>> +
>>>> +# 30s is enough to trigger bug
>>>> +sleep $((30*$TIME_FACTOR))
>>>> +kill $fsstress_pid $balance_pid
>>>> +wait
>>>> +
>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>> balance cancel
>>>> +# to cancel running or paused balance.
>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>> +
>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>> +
>>>> +_scratch_unmount
>>>> +
>>>> +# generate a qgroup report and look for inconsistent groups
>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>> +if [ $? -ne 0 ]; then
>>>> +    echo "Silence is golden"
>>>> +    # success, all done
>>>> +    status=0
>>>> +fi
>>>
>>>
>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>>> which means btrfs check finds inconsistent groups. But according to your
>>> commit log, current kernel should pass the test. So is the failure
>>> expected?
>>>
>>> Also, just grep for different qgroup counts and print the message out if
>>> grep finds the message, so it breaks golden image on error and we know
>>> something really goes wrong. Right now test fails just because of
>>> missing "Silence is golden", which is unclear why it fails:
>>>
>>>  @@ -1,2 +1 @@
>>>   QA output created by 028
>>>  -Silence is golden
>>>
>>> Do the following instead:
>>>
>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>     echo "Silence is golden"
>>>     status=0
>>>     exit
>>>
>>> And we see this on failure:
>>>
>>>  @@ -1,2 +1,3 @@
>>>   QA output created by 028
>>>  +Counts for qgroup id: 5 are different
>>>   Silence is golden
>>>
>>> Thanks,
>>> Eryu
>>>
>>>> +
>>>> +exit
>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>> new file mode 100644
>>>> index 0000000..2615f73
>>>> --- /dev/null
>>>> +++ b/tests/btrfs/028.out
>>>> @@ -0,0 +1,2 @@
>>>> +QA output created by 028
>>>> +Silence is golden
>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>> index da0e27f..35ecf59 100644
>>>> --- a/tests/btrfs/group
>>>> +++ b/tests/btrfs/group
>>>> @@ -30,6 +30,7 @@
>>>>  025 auto quick send clone
>>>>  026 auto quick compress prealloc
>>>>  027 auto replace
>>>> +028 auto qgroup balance
>>>>  029 auto quick clone
>>>>  030 auto quick send
>>>>  031 auto quick subvol clone
>>>> --
>>>> 2.5.5
>>>>
>>>>
>>>>
>>>> --
>>>> 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
>>>
>>>
>>>
>>
>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
>> 4.7-rc3), the test didn't fail. Can you provide me with more information
>> about your test environment?
>>
>>
> Sorry, this test fails after I perform it more times. It seems to be a bug
> of btrfs qgroup. We will continue to track this bug.

Which is already known and reported some time ago:

http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824

It's another regression introduced in kernel 4.2 with the large
qgroups rewrite (data extents are not accounted anymore after
balance).
It's trivial to reproduce with:


#!/bin/bash -x

MNT="/mnt/sdi"
DEV="/dev/sdi"

mkfs.btrfs --nodesize=4096 -f $DEV
mount -t btrfs $DEV $MNT

# fill metadata space, create a tree with 1 node and 2 leaves
for ((i = 1; i <= 10; i++)); do
        mkdir $MNT/d$i
done
for ((i = 1; i <= 1; i++)); do
        xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
done

btrfs fi sync $MNT
btrfs quota enable $MNT
btrfs quota rescan -w $MNT
btrfs qg show $MNT

umount $MNT
btrfsck $DEV

mount -t btrfs $DEV $MNT

time btrfs balance start -d $MNT

umount $MNT
btrfsck $DEV


Also, please, when you send a new version of a patch, add a changelog
describing what changed between versions. See [1] for an example.
I noticed you do this for all patches you send.

[1] https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions

>
>
> --
> Thanks,
> Lu
>
>
> --
> 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



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Qu Wenruo]

At 06/13/2016 05:49 PM, Filipe Manana wrote:
> On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com> wrote:
>> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>>
>>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>>
>>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>>
>>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>>> "extent de-reference" means that reducing an extent's reference count
>>>>> or freeing an extent.
>>>>> Although current qgroup can handle it, we still need to prevent any
>>>>> regression which may break current qgroup.
>>>>>
>>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>  common/rc           |  4 +--
>>>>>  tests/btrfs/028     | 98
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  tests/btrfs/028.out |  2 ++
>>>>>  tests/btrfs/group   |  1 +
>>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>>  create mode 100755 tests/btrfs/028
>>>>>  create mode 100644 tests/btrfs/028.out
>>>>>
>>>>> diff --git a/common/rc b/common/rc
>>>>> index 51092a0..650d198 100644
>>>>> --- a/common/rc
>>>>> +++ b/common/rc
>>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>>  # stress btrfs by running balance operation in a loop
>>>>>  _btrfs_stress_balance()
>>>>>  {
>>>>> -    local btrfs_mnt=$1
>>>>> +    local options=$@
>>>>>      while true; do
>>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>>      done
>>>>>  }
>>>>>
>>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>>> new file mode 100755
>>>>> index 0000000..8cea49a
>>>>> --- /dev/null
>>>>> +++ b/tests/btrfs/028
>>>>> @@ -0,0 +1,98 @@
>>>>> +#! /bin/bash
>>>>> +# FS QA Test 028
>>>>> +#
>>>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>>>> +# "extent de-reference" means that reducing an extent's reference count
>>>>> +# or freeing an extent.
>>>>> +# Although current qgroup can handle it, we still need to prevent any
>>>>> +# regression which may break current qgroup.
>>>>> +#
>>>>>
>>>>> +#-----------------------------------------------------------------------
>>>>>
>>>>> +# Copyright (c) 2016 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!
>>>>> +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
>>>>> +_supported_fs btrfs
>>>>> +_supported_os Linux
>>>>> +_require_scratch
>>>>> +
>>>>> +_scratch_mkfs
>>>>> +_scratch_mount
>>>>> +
>>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>>> +
>>>>> +# Increase the probability of generating de-refer extent, and decrease
>>>>> +# other.
>>>>> +args=`_scale_fsstress_args -z \
>>>>> +    -f write=10 -f unlink=10 \
>>>>> +    -f creat=10 -f fsync=10 \
>>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>>> +echo "Run fsstress $args" >>$seqres.full
>>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>>> +fsstress_pid=$!
>>>>> +
>>>>> +echo "Start balance" >>$seqres.full
>>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>>> +balance_pid=$!
>>>>> +
>>>>> +# 30s is enough to trigger bug
>>>>> +sleep $((30*$TIME_FACTOR))
>>>>> +kill $fsstress_pid $balance_pid
>>>>> +wait
>>>>> +
>>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>>> balance cancel
>>>>> +# to cancel running or paused balance.
>>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>>> +
>>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>>> +
>>>>> +_scratch_unmount
>>>>> +
>>>>> +# generate a qgroup report and look for inconsistent groups
>>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>>> +if [ $? -ne 0 ]; then
>>>>> +    echo "Silence is golden"
>>>>> +    # success, all done
>>>>> +    status=0
>>>>> +fi
>>>>
>>>>
>>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>>>> which means btrfs check finds inconsistent groups. But according to your
>>>> commit log, current kernel should pass the test. So is the failure
>>>> expected?
>>>>
>>>> Also, just grep for different qgroup counts and print the message out if
>>>> grep finds the message, so it breaks golden image on error and we know
>>>> something really goes wrong. Right now test fails just because of
>>>> missing "Silence is golden", which is unclear why it fails:
>>>>
>>>>  @@ -1,2 +1 @@
>>>>   QA output created by 028
>>>>  -Silence is golden
>>>>
>>>> Do the following instead:
>>>>
>>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>>     echo "Silence is golden"
>>>>     status=0
>>>>     exit
>>>>
>>>> And we see this on failure:
>>>>
>>>>  @@ -1,2 +1,3 @@
>>>>   QA output created by 028
>>>>  +Counts for qgroup id: 5 are different
>>>>   Silence is golden
>>>>
>>>> Thanks,
>>>> Eryu
>>>>
>>>>> +
>>>>> +exit
>>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>>> new file mode 100644
>>>>> index 0000000..2615f73
>>>>> --- /dev/null
>>>>> +++ b/tests/btrfs/028.out
>>>>> @@ -0,0 +1,2 @@
>>>>> +QA output created by 028
>>>>> +Silence is golden
>>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>>> index da0e27f..35ecf59 100644
>>>>> --- a/tests/btrfs/group
>>>>> +++ b/tests/btrfs/group
>>>>> @@ -30,6 +30,7 @@
>>>>>  025 auto quick send clone
>>>>>  026 auto quick compress prealloc
>>>>>  027 auto replace
>>>>> +028 auto qgroup balance
>>>>>  029 auto quick clone
>>>>>  030 auto quick send
>>>>>  031 auto quick subvol clone
>>>>> --
>>>>> 2.5.5
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>>
>>>>
>>>
>>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
>>> 4.7-rc3), the test didn't fail. Can you provide me with more information
>>> about your test environment?
>>>
>>>
>> Sorry, this test fails after I perform it more times. It seems to be a bug
>> of btrfs qgroup. We will continue to track this bug.
>
> Which is already known and reported some time ago:
>
> http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824
>
> It's another regression introduced in kernel 4.2 with the large
> qgroups rewrite (data extents are not accounted anymore after
> balance).
> It's trivial to reproduce with:
>
>
> #!/bin/bash -x
>
> MNT="/mnt/sdi"
> DEV="/dev/sdi"
>
> mkfs.btrfs --nodesize=4096 -f $DEV
> mount -t btrfs $DEV $MNT
>
> # fill metadata space, create a tree with 1 node and 2 leaves
> for ((i = 1; i <= 10; i++)); do
>         mkdir $MNT/d$i
> done
> for ((i = 1; i <= 1; i++)); do
>         xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
> done
>
> btrfs fi sync $MNT
> btrfs quota enable $MNT
> btrfs quota rescan -w $MNT
> btrfs qg show $MNT
>
> umount $MNT
> btrfsck $DEV
>
> mount -t btrfs $DEV $MNT
>
> time btrfs balance start -d $MNT
>
> umount $MNT
> btrfsck $DEV
Yes, we also found this internally.

And the direct cause is already located.

The direct cause is, we're increasing new data extents' reference even 
the referencer(fs tree) is still referring to old extents.

If using ftrace to trace add/run_delayed_data_ref() and 
qgroup_account_extent(), one should find that, new data extents' 
reference is increased with both data reloc tree and final 
referencer.(fs tree in the test script).

In that timing, backref walk can't find any root referring the new data 
extents.
The data reloc tree is the current referencer, but qgroup won't account 
data reloc tree.
While new data extents have fs tree backref, fs tree still refers to the 
old extent. So backref walk won't count it either.

And the commit_trans before we really merges reloc roots makes the 
accounting wrong.

So it seems to be an existing problem before the qgroup rework.

The quick fix won't be hard, just re-dirty the new data extent at 
merge_reloc_roots() time.

But I'm still investigating if we can fix it in a better method, to 
increase the new data extents' reference at correct time.

Thanks,
Qu

>
>
> Also, please, when you send a new version of a patch, add a changelog
> describing what changed between versions. See [1] for an example.
> I noticed you do this for all patches you send.
>
> [1] https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions
>
>>
>>
>> --
>> Thanks,
>> Lu
>>
>>
>> --
>> 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 v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Lu Fengqi]

At 06/13/2016 05:49 PM, Filipe Manana wrote:
> On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com> wrote:
>> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>>
>>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>>
>>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>>
>>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>>> "extent de-reference" means that reducing an extent's reference count
>>>>> or freeing an extent.
>>>>> Although current qgroup can handle it, we still need to prevent any
>>>>> regression which may break current qgroup.
>>>>>
>>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>  common/rc           |  4 +--
>>>>>  tests/btrfs/028     | 98
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  tests/btrfs/028.out |  2 ++
>>>>>  tests/btrfs/group   |  1 +
>>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>>  create mode 100755 tests/btrfs/028
>>>>>  create mode 100644 tests/btrfs/028.out
>>>>>
>>>>> diff --git a/common/rc b/common/rc
>>>>> index 51092a0..650d198 100644
>>>>> --- a/common/rc
>>>>> +++ b/common/rc
>>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>>  # stress btrfs by running balance operation in a loop
>>>>>  _btrfs_stress_balance()
>>>>>  {
>>>>> -    local btrfs_mnt=$1
>>>>> +    local options=$@
>>>>>      while true; do
>>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>>      done
>>>>>  }
>>>>>
>>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>>> new file mode 100755
>>>>> index 0000000..8cea49a
>>>>> --- /dev/null
>>>>> +++ b/tests/btrfs/028
>>>>> @@ -0,0 +1,98 @@
>>>>> +#! /bin/bash
>>>>> +# FS QA Test 028
>>>>> +#
>>>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>>>> +# "extent de-reference" means that reducing an extent's reference count
>>>>> +# or freeing an extent.
>>>>> +# Although current qgroup can handle it, we still need to prevent any
>>>>> +# regression which may break current qgroup.
>>>>> +#
>>>>>
>>>>> +#-----------------------------------------------------------------------
>>>>>
>>>>> +# Copyright (c) 2016 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!
>>>>> +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
>>>>> +_supported_fs btrfs
>>>>> +_supported_os Linux
>>>>> +_require_scratch
>>>>> +
>>>>> +_scratch_mkfs
>>>>> +_scratch_mount
>>>>> +
>>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>>> +
>>>>> +# Increase the probability of generating de-refer extent, and decrease
>>>>> +# other.
>>>>> +args=`_scale_fsstress_args -z \
>>>>> +    -f write=10 -f unlink=10 \
>>>>> +    -f creat=10 -f fsync=10 \
>>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>>> +echo "Run fsstress $args" >>$seqres.full
>>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>>> +fsstress_pid=$!
>>>>> +
>>>>> +echo "Start balance" >>$seqres.full
>>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>>> +balance_pid=$!
>>>>> +
>>>>> +# 30s is enough to trigger bug
>>>>> +sleep $((30*$TIME_FACTOR))
>>>>> +kill $fsstress_pid $balance_pid
>>>>> +wait
>>>>> +
>>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>>> balance cancel
>>>>> +# to cancel running or paused balance.
>>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>>> +
>>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>>> +
>>>>> +_scratch_unmount
>>>>> +
>>>>> +# generate a qgroup report and look for inconsistent groups
>>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>>> +if [ $? -ne 0 ]; then
>>>>> +    echo "Silence is golden"
>>>>> +    # success, all done
>>>>> +    status=0
>>>>> +fi
>>>>
>>>>
>>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>>>> which means btrfs check finds inconsistent groups. But according to your
>>>> commit log, current kernel should pass the test. So is the failure
>>>> expected?
>>>>
>>>> Also, just grep for different qgroup counts and print the message out if
>>>> grep finds the message, so it breaks golden image on error and we know
>>>> something really goes wrong. Right now test fails just because of
>>>> missing "Silence is golden", which is unclear why it fails:
>>>>
>>>>  @@ -1,2 +1 @@
>>>>   QA output created by 028
>>>>  -Silence is golden
>>>>
>>>> Do the following instead:
>>>>
>>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>>     echo "Silence is golden"
>>>>     status=0
>>>>     exit
>>>>
>>>> And we see this on failure:
>>>>
>>>>  @@ -1,2 +1,3 @@
>>>>   QA output created by 028
>>>>  +Counts for qgroup id: 5 are different
>>>>   Silence is golden
>>>>
>>>> Thanks,
>>>> Eryu
>>>>
>>>>> +
>>>>> +exit
>>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>>> new file mode 100644
>>>>> index 0000000..2615f73
>>>>> --- /dev/null
>>>>> +++ b/tests/btrfs/028.out
>>>>> @@ -0,0 +1,2 @@
>>>>> +QA output created by 028
>>>>> +Silence is golden
>>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>>> index da0e27f..35ecf59 100644
>>>>> --- a/tests/btrfs/group
>>>>> +++ b/tests/btrfs/group
>>>>> @@ -30,6 +30,7 @@
>>>>>  025 auto quick send clone
>>>>>  026 auto quick compress prealloc
>>>>>  027 auto replace
>>>>> +028 auto qgroup balance
>>>>>  029 auto quick clone
>>>>>  030 auto quick send
>>>>>  031 auto quick subvol clone
>>>>> --
>>>>> 2.5.5
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>>
>>>>
>>>
>>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
>>> 4.7-rc3), the test didn't fail. Can you provide me with more information
>>> about your test environment?
>>>
>>>
>> Sorry, this test fails after I perform it more times. It seems to be a bug
>> of btrfs qgroup. We will continue to track this bug.
>
> Which is already known and reported some time ago:
>
> http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824
>
> It's another regression introduced in kernel 4.2 with the large
> qgroups rewrite (data extents are not accounted anymore after
> balance).
> It's trivial to reproduce with:
>
>
> #!/bin/bash -x
>
> MNT="/mnt/sdi"
> DEV="/dev/sdi"
>
> mkfs.btrfs --nodesize=4096 -f $DEV
> mount -t btrfs $DEV $MNT
>
> # fill metadata space, create a tree with 1 node and 2 leaves
> for ((i = 1; i <= 10; i++)); do
>         mkdir $MNT/d$i
> done
> for ((i = 1; i <= 1; i++)); do
>         xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
> done
>
> btrfs fi sync $MNT
> btrfs quota enable $MNT
> btrfs quota rescan -w $MNT
> btrfs qg show $MNT
>
> umount $MNT
> btrfsck $DEV
>
> mount -t btrfs $DEV $MNT
>
> time btrfs balance start -d $MNT
>
> umount $MNT
> btrfsck $DEV
>
OK.
>
> Also, please, when you send a new version of a patch, add a changelog
> describing what changed between versions. See [1] for an example.
> I noticed you do this for all patches you send.
>
> [1] https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions
>
Sorry, I will add the change log when I update this patch.


-- 
Thanks,
Lu

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Filipe Manana]

On Tue, Jun 14, 2016 at 1:58 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>
>
> At 06/13/2016 05:49 PM, Filipe Manana wrote:
>>
>> On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>> wrote:
>>>
>>> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>>>
>>>>
>>>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>>>
>>>>>
>>>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>>>
>>>>>>
>>>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>>>> "extent de-reference" means that reducing an extent's reference count
>>>>>> or freeing an extent.
>>>>>> Although current qgroup can handle it, we still need to prevent any
>>>>>> regression which may break current qgroup.
>>>>>>
>>>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>>  common/rc           |  4 +--
>>>>>>  tests/btrfs/028     | 98
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  tests/btrfs/028.out |  2 ++
>>>>>>  tests/btrfs/group   |  1 +
>>>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>>>  create mode 100755 tests/btrfs/028
>>>>>>  create mode 100644 tests/btrfs/028.out
>>>>>>
>>>>>> diff --git a/common/rc b/common/rc
>>>>>> index 51092a0..650d198 100644
>>>>>> --- a/common/rc
>>>>>> +++ b/common/rc
>>>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>>>  # stress btrfs by running balance operation in a loop
>>>>>>  _btrfs_stress_balance()
>>>>>>  {
>>>>>> -    local btrfs_mnt=$1
>>>>>> +    local options=$@
>>>>>>      while true; do
>>>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>>>      done
>>>>>>  }
>>>>>>
>>>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>>>> new file mode 100755
>>>>>> index 0000000..8cea49a
>>>>>> --- /dev/null
>>>>>> +++ b/tests/btrfs/028
>>>>>> @@ -0,0 +1,98 @@
>>>>>> +#! /bin/bash
>>>>>> +# FS QA Test 028
>>>>>> +#
>>>>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>>>>> +# "extent de-reference" means that reducing an extent's reference
>>>>>> count
>>>>>> +# or freeing an extent.
>>>>>> +# Although current qgroup can handle it, we still need to prevent any
>>>>>> +# regression which may break current qgroup.
>>>>>> +#
>>>>>>
>>>>>>
>>>>>> +#-----------------------------------------------------------------------
>>>>>>
>>>>>> +# Copyright (c) 2016 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!
>>>>>> +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
>>>>>> +_supported_fs btrfs
>>>>>> +_supported_os Linux
>>>>>> +_require_scratch
>>>>>> +
>>>>>> +_scratch_mkfs
>>>>>> +_scratch_mount
>>>>>> +
>>>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>>>> +
>>>>>> +# Increase the probability of generating de-refer extent, and
>>>>>> decrease
>>>>>> +# other.
>>>>>> +args=`_scale_fsstress_args -z \
>>>>>> +    -f write=10 -f unlink=10 \
>>>>>> +    -f creat=10 -f fsync=10 \
>>>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>>>> +echo "Run fsstress $args" >>$seqres.full
>>>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>>>> +fsstress_pid=$!
>>>>>> +
>>>>>> +echo "Start balance" >>$seqres.full
>>>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>>>> +balance_pid=$!
>>>>>> +
>>>>>> +# 30s is enough to trigger bug
>>>>>> +sleep $((30*$TIME_FACTOR))
>>>>>> +kill $fsstress_pid $balance_pid
>>>>>> +wait
>>>>>> +
>>>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>>>> balance cancel
>>>>>> +# to cancel running or paused balance.
>>>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>>>> +
>>>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>>>> +
>>>>>> +_scratch_unmount
>>>>>> +
>>>>>> +# generate a qgroup report and look for inconsistent groups
>>>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>>>> +if [ $? -ne 0 ]; then
>>>>>> +    echo "Silence is golden"
>>>>>> +    # success, all done
>>>>>> +    status=0
>>>>>> +fi
>>>>>
>>>>>
>>>>>
>>>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>>>>> which means btrfs check finds inconsistent groups. But according to
>>>>> your
>>>>> commit log, current kernel should pass the test. So is the failure
>>>>> expected?
>>>>>
>>>>> Also, just grep for different qgroup counts and print the message out
>>>>> if
>>>>> grep finds the message, so it breaks golden image on error and we know
>>>>> something really goes wrong. Right now test fails just because of
>>>>> missing "Silence is golden", which is unclear why it fails:
>>>>>
>>>>>  @@ -1,2 +1 @@
>>>>>   QA output created by 028
>>>>>  -Silence is golden
>>>>>
>>>>> Do the following instead:
>>>>>
>>>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>>>     echo "Silence is golden"
>>>>>     status=0
>>>>>     exit
>>>>>
>>>>> And we see this on failure:
>>>>>
>>>>>  @@ -1,2 +1,3 @@
>>>>>   QA output created by 028
>>>>>  +Counts for qgroup id: 5 are different
>>>>>   Silence is golden
>>>>>
>>>>> Thanks,
>>>>> Eryu
>>>>>
>>>>>> +
>>>>>> +exit
>>>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>>>> new file mode 100644
>>>>>> index 0000000..2615f73
>>>>>> --- /dev/null
>>>>>> +++ b/tests/btrfs/028.out
>>>>>> @@ -0,0 +1,2 @@
>>>>>> +QA output created by 028
>>>>>> +Silence is golden
>>>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>>>> index da0e27f..35ecf59 100644
>>>>>> --- a/tests/btrfs/group
>>>>>> +++ b/tests/btrfs/group
>>>>>> @@ -30,6 +30,7 @@
>>>>>>  025 auto quick send clone
>>>>>>  026 auto quick compress prealloc
>>>>>>  027 auto replace
>>>>>> +028 auto qgroup balance
>>>>>>  029 auto quick clone
>>>>>>  030 auto quick send
>>>>>>  031 auto quick subvol clone
>>>>>> --
>>>>>> 2.5.5
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> 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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
>>>> 4.7-rc3), the test didn't fail. Can you provide me with more information
>>>> about your test environment?
>>>>
>>>>
>>> Sorry, this test fails after I perform it more times. It seems to be a
>>> bug
>>> of btrfs qgroup. We will continue to track this bug.
>>
>>
>> Which is already known and reported some time ago:
>>
>> http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824
>>
>> It's another regression introduced in kernel 4.2 with the large
>> qgroups rewrite (data extents are not accounted anymore after
>> balance).
>> It's trivial to reproduce with:
>>
>>
>> #!/bin/bash -x
>>
>> MNT="/mnt/sdi"
>> DEV="/dev/sdi"
>>
>> mkfs.btrfs --nodesize=4096 -f $DEV
>> mount -t btrfs $DEV $MNT
>>
>> # fill metadata space, create a tree with 1 node and 2 leaves
>> for ((i = 1; i <= 10; i++)); do
>>         mkdir $MNT/d$i
>> done
>> for ((i = 1; i <= 1; i++)); do
>>         xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
>> done
>>
>> btrfs fi sync $MNT
>> btrfs quota enable $MNT
>> btrfs quota rescan -w $MNT
>> btrfs qg show $MNT
>>
>> umount $MNT
>> btrfsck $DEV
>>
>> mount -t btrfs $DEV $MNT
>>
>> time btrfs balance start -d $MNT
>>
>> umount $MNT
>> btrfsck $DEV
>
> Yes, we also found this internally.
>
> And the direct cause is already located.
>
> The direct cause is, we're increasing new data extents' reference even the
> referencer(fs tree) is still referring to old extents.
>
> If using ftrace to trace add/run_delayed_data_ref() and
> qgroup_account_extent(), one should find that, new data extents' reference
> is increased with both data reloc tree and final referencer.(fs tree in the
> test script).
>
> In that timing, backref walk can't find any root referring the new data
> extents.
> The data reloc tree is the current referencer, but qgroup won't account data
> reloc tree.
> While new data extents have fs tree backref, fs tree still refers to the old
> extent. So backref walk won't count it either.
>
> And the commit_trans before we really merges reloc roots makes the
> accounting wrong.
>
> So it seems to be an existing problem before the qgroup rework.

How can it be an existing problem before the qgroup rework if this
example (and others more complex involving snapshots and a large
number of data extents) worked on 4.1 and previous kernels?



>
> The quick fix won't be hard, just re-dirty the new data extent at
> merge_reloc_roots() time.
>
> But I'm still investigating if we can fix it in a better method, to increase
> the new data extents' reference at correct time.
>
> Thanks,
> Qu
>
>
>>
>>
>> Also, please, when you send a new version of a patch, add a changelog
>> describing what changed between versions. See [1] for an example.
>> I noticed you do this for all patches you send.
>>
>> [1]
>> https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions
>>
>>>
>>>
>>> --
>>> Thanks,
>>> Lu
>>>
>>>
>>> --
>>> 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
>>
>>
>>
>>
>
>



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Qu Wenruo]

At 06/14/2016 04:41 PM, Filipe Manana wrote:
> On Tue, Jun 14, 2016 at 1:58 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>>
>>
>> At 06/13/2016 05:49 PM, Filipe Manana wrote:
>>>
>>> On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>> wrote:
>>>>
>>>> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>>>>
>>>>>
>>>>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>>>>
>>>>>>>
>>>>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>>>>> "extent de-reference" means that reducing an extent's reference count
>>>>>>> or freeing an extent.
>>>>>>> Although current qgroup can handle it, we still need to prevent any
>>>>>>> regression which may break current qgroup.
>>>>>>>
>>>>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>>>>> ---
>>>>>>>  common/rc           |  4 +--
>>>>>>>  tests/btrfs/028     | 98
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>  tests/btrfs/028.out |  2 ++
>>>>>>>  tests/btrfs/group   |  1 +
>>>>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>>>>  create mode 100755 tests/btrfs/028
>>>>>>>  create mode 100644 tests/btrfs/028.out
>>>>>>>
>>>>>>> diff --git a/common/rc b/common/rc
>>>>>>> index 51092a0..650d198 100644
>>>>>>> --- a/common/rc
>>>>>>> +++ b/common/rc
>>>>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>>>>  # stress btrfs by running balance operation in a loop
>>>>>>>  _btrfs_stress_balance()
>>>>>>>  {
>>>>>>> -    local btrfs_mnt=$1
>>>>>>> +    local options=$@
>>>>>>>      while true; do
>>>>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>>>>      done
>>>>>>>  }
>>>>>>>
>>>>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>>>>> new file mode 100755
>>>>>>> index 0000000..8cea49a
>>>>>>> --- /dev/null
>>>>>>> +++ b/tests/btrfs/028
>>>>>>> @@ -0,0 +1,98 @@
>>>>>>> +#! /bin/bash
>>>>>>> +# FS QA Test 028
>>>>>>> +#
>>>>>>> +# Test if qgroup can handle extent de-reference during reallocation.
>>>>>>> +# "extent de-reference" means that reducing an extent's reference
>>>>>>> count
>>>>>>> +# or freeing an extent.
>>>>>>> +# Although current qgroup can handle it, we still need to prevent any
>>>>>>> +# regression which may break current qgroup.
>>>>>>> +#
>>>>>>>
>>>>>>>
>>>>>>> +#-----------------------------------------------------------------------
>>>>>>>
>>>>>>> +# Copyright (c) 2016 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!
>>>>>>> +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
>>>>>>> +_supported_fs btrfs
>>>>>>> +_supported_os Linux
>>>>>>> +_require_scratch
>>>>>>> +
>>>>>>> +_scratch_mkfs
>>>>>>> +_scratch_mount
>>>>>>> +
>>>>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>>>>> +
>>>>>>> +# Increase the probability of generating de-refer extent, and
>>>>>>> decrease
>>>>>>> +# other.
>>>>>>> +args=`_scale_fsstress_args -z \
>>>>>>> +    -f write=10 -f unlink=10 \
>>>>>>> +    -f creat=10 -f fsync=10 \
>>>>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>>>>> +echo "Run fsstress $args" >>$seqres.full
>>>>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>>>>> +fsstress_pid=$!
>>>>>>> +
>>>>>>> +echo "Start balance" >>$seqres.full
>>>>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>>>>> +balance_pid=$!
>>>>>>> +
>>>>>>> +# 30s is enough to trigger bug
>>>>>>> +sleep $((30*$TIME_FACTOR))
>>>>>>> +kill $fsstress_pid $balance_pid
>>>>>>> +wait
>>>>>>> +
>>>>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>>>>> balance cancel
>>>>>>> +# to cancel running or paused balance.
>>>>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>>>>> +
>>>>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>>>>> +
>>>>>>> +_scratch_unmount
>>>>>>> +
>>>>>>> +# generate a qgroup report and look for inconsistent groups
>>>>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>>>>> +if [ $? -ne 0 ]; then
>>>>>>> +    echo "Silence is golden"
>>>>>>> +    # success, all done
>>>>>>> +    status=0
>>>>>>> +fi
>>>>>>
>>>>>>
>>>>>>
>>>>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test fails,
>>>>>> which means btrfs check finds inconsistent groups. But according to
>>>>>> your
>>>>>> commit log, current kernel should pass the test. So is the failure
>>>>>> expected?
>>>>>>
>>>>>> Also, just grep for different qgroup counts and print the message out
>>>>>> if
>>>>>> grep finds the message, so it breaks golden image on error and we know
>>>>>> something really goes wrong. Right now test fails just because of
>>>>>> missing "Silence is golden", which is unclear why it fails:
>>>>>>
>>>>>>  @@ -1,2 +1 @@
>>>>>>   QA output created by 028
>>>>>>  -Silence is golden
>>>>>>
>>>>>> Do the following instead:
>>>>>>
>>>>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>>>>     echo "Silence is golden"
>>>>>>     status=0
>>>>>>     exit
>>>>>>
>>>>>> And we see this on failure:
>>>>>>
>>>>>>  @@ -1,2 +1,3 @@
>>>>>>   QA output created by 028
>>>>>>  +Counts for qgroup id: 5 are different
>>>>>>   Silence is golden
>>>>>>
>>>>>> Thanks,
>>>>>> Eryu
>>>>>>
>>>>>>> +
>>>>>>> +exit
>>>>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>>>>> new file mode 100644
>>>>>>> index 0000000..2615f73
>>>>>>> --- /dev/null
>>>>>>> +++ b/tests/btrfs/028.out
>>>>>>> @@ -0,0 +1,2 @@
>>>>>>> +QA output created by 028
>>>>>>> +Silence is golden
>>>>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>>>>> index da0e27f..35ecf59 100644
>>>>>>> --- a/tests/btrfs/group
>>>>>>> +++ b/tests/btrfs/group
>>>>>>> @@ -30,6 +30,7 @@
>>>>>>>  025 auto quick send clone
>>>>>>>  026 auto quick compress prealloc
>>>>>>>  027 auto replace
>>>>>>> +028 auto qgroup balance
>>>>>>>  029 auto quick clone
>>>>>>>  030 auto quick send
>>>>>>>  031 auto quick subvol clone
>>>>>>> --
>>>>>>> 2.5.5
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> 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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1 /
>>>>> 4.7-rc3), the test didn't fail. Can you provide me with more information
>>>>> about your test environment?
>>>>>
>>>>>
>>>> Sorry, this test fails after I perform it more times. It seems to be a
>>>> bug
>>>> of btrfs qgroup. We will continue to track this bug.
>>>
>>>
>>> Which is already known and reported some time ago:
>>>
>>> http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824
>>>
>>> It's another regression introduced in kernel 4.2 with the large
>>> qgroups rewrite (data extents are not accounted anymore after
>>> balance).
>>> It's trivial to reproduce with:
>>>
>>>
>>> #!/bin/bash -x
>>>
>>> MNT="/mnt/sdi"
>>> DEV="/dev/sdi"
>>>
>>> mkfs.btrfs --nodesize=4096 -f $DEV
>>> mount -t btrfs $DEV $MNT
>>>
>>> # fill metadata space, create a tree with 1 node and 2 leaves
>>> for ((i = 1; i <= 10; i++)); do
>>>         mkdir $MNT/d$i
>>> done
>>> for ((i = 1; i <= 1; i++)); do
>>>         xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
>>> done
>>>
>>> btrfs fi sync $MNT
>>> btrfs quota enable $MNT
>>> btrfs quota rescan -w $MNT
>>> btrfs qg show $MNT
>>>
>>> umount $MNT
>>> btrfsck $DEV
>>>
>>> mount -t btrfs $DEV $MNT
>>>
>>> time btrfs balance start -d $MNT
>>>
>>> umount $MNT
>>> btrfsck $DEV
>>
>> Yes, we also found this internally.
>>
>> And the direct cause is already located.
>>
>> The direct cause is, we're increasing new data extents' reference even the
>> referencer(fs tree) is still referring to old extents.
>>
>> If using ftrace to trace add/run_delayed_data_ref() and
>> qgroup_account_extent(), one should find that, new data extents' reference
>> is increased with both data reloc tree and final referencer.(fs tree in the
>> test script).
>>
>> In that timing, backref walk can't find any root referring the new data
>> extents.
>> The data reloc tree is the current referencer, but qgroup won't account data
>> reloc tree.
>> While new data extents have fs tree backref, fs tree still refers to the old
>> extent. So backref walk won't count it either.
>>
>> And the commit_trans before we really merges reloc roots makes the
>> accounting wrong.
>>
>> So it seems to be an existing problem before the qgroup rework.
>
> How can it be an existing problem before the qgroup rework if this
> example (and others more complex involving snapshots and a large
> number of data extents) worked on 4.1 and previous kernels?

Because in old kernels, backref walk is unreliable, especially for such 
tricky tree block swap thing.

As you can see, it's very obvious that, while fs tree is still referring 
to the old extent, it's never possible to get nr_old/new_roots to 
anything other than 0 for new extent.

So for old kernels, either backref walk got wrong(as it just happend), 
or it doesn't go through qgroup accounting at all.

Thanks,
Qu

>
>
>
>>
>> The quick fix won't be hard, just re-dirty the new data extent at
>> merge_reloc_roots() time.
>>
>> But I'm still investigating if we can fix it in a better method, to increase
>> the new data extents' reference at correct time.
>>
>> Thanks,
>> Qu
>>
>>
>>>
>>>
>>> Also, please, when you send a new version of a patch, add a changelog
>>> describing what changed between versions. See [1] for an example.
>>> I noticed you do this for all patches you send.
>>>
>>> [1]
>>> https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions
>>>
>>>>
>>>>
>>>> --
>>>> Thanks,
>>>> Lu
>>>>
>>>>
>>>> --
>>>> 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 v3] fstests: btrfs: add test for qgroup handle extent de-reference

[Filipe Manana]

On Tue, Jun 14, 2016 at 10:08 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>
>
> At 06/14/2016 04:41 PM, Filipe Manana wrote:
>>
>> On Tue, Jun 14, 2016 at 1:58 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>> wrote:
>>>
>>>
>>>
>>> At 06/13/2016 05:49 PM, Filipe Manana wrote:
>>>>
>>>>
>>>> On Mon, Jun 13, 2016 at 9:06 AM, Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> At 06/13/2016 03:29 PM, Lu Fengqi wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> At 06/13/2016 11:04 AM, Eryu Guan wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 13, 2016 at 10:10:50AM +0800, Lu Fengqi wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Test if qgroup can handle extent de-reference during reallocation.
>>>>>>>> "extent de-reference" means that reducing an extent's reference
>>>>>>>> count
>>>>>>>> or freeing an extent.
>>>>>>>> Although current qgroup can handle it, we still need to prevent any
>>>>>>>> regression which may break current qgroup.
>>>>>>>>
>>>>>>>> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
>>>>>>>> ---
>>>>>>>>  common/rc           |  4 +--
>>>>>>>>  tests/btrfs/028     | 98
>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>  tests/btrfs/028.out |  2 ++
>>>>>>>>  tests/btrfs/group   |  1 +
>>>>>>>>  4 files changed, 103 insertions(+), 2 deletions(-)
>>>>>>>>  create mode 100755 tests/btrfs/028
>>>>>>>>  create mode 100644 tests/btrfs/028.out
>>>>>>>>
>>>>>>>> diff --git a/common/rc b/common/rc
>>>>>>>> index 51092a0..650d198 100644
>>>>>>>> --- a/common/rc
>>>>>>>> +++ b/common/rc
>>>>>>>> @@ -3284,9 +3284,9 @@ _btrfs_get_profile_configs()
>>>>>>>>  # stress btrfs by running balance operation in a loop
>>>>>>>>  _btrfs_stress_balance()
>>>>>>>>  {
>>>>>>>> -    local btrfs_mnt=$1
>>>>>>>> +    local options=$@
>>>>>>>>      while true; do
>>>>>>>> -        $BTRFS_UTIL_PROG balance start $btrfs_mnt
>>>>>>>> +        $BTRFS_UTIL_PROG balance start $options
>>>>>>>>      done
>>>>>>>>  }
>>>>>>>>
>>>>>>>> diff --git a/tests/btrfs/028 b/tests/btrfs/028
>>>>>>>> new file mode 100755
>>>>>>>> index 0000000..8cea49a
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/tests/btrfs/028
>>>>>>>> @@ -0,0 +1,98 @@
>>>>>>>> +#! /bin/bash
>>>>>>>> +# FS QA Test 028
>>>>>>>> +#
>>>>>>>> +# Test if qgroup can handle extent de-reference during
>>>>>>>> reallocation.
>>>>>>>> +# "extent de-reference" means that reducing an extent's reference
>>>>>>>> count
>>>>>>>> +# or freeing an extent.
>>>>>>>> +# Although current qgroup can handle it, we still need to prevent
>>>>>>>> any
>>>>>>>> +# regression which may break current qgroup.
>>>>>>>> +#
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> +#-----------------------------------------------------------------------
>>>>>>>>
>>>>>>>> +# Copyright (c) 2016 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!
>>>>>>>> +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
>>>>>>>> +_supported_fs btrfs
>>>>>>>> +_supported_os Linux
>>>>>>>> +_require_scratch
>>>>>>>> +
>>>>>>>> +_scratch_mkfs
>>>>>>>> +_scratch_mount
>>>>>>>> +
>>>>>>>> +_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>>>>>>> +_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>>>>>>>> +
>>>>>>>> +# Increase the probability of generating de-refer extent, and
>>>>>>>> decrease
>>>>>>>> +# other.
>>>>>>>> +args=`_scale_fsstress_args -z \
>>>>>>>> +    -f write=10 -f unlink=10 \
>>>>>>>> +    -f creat=10 -f fsync=10 \
>>>>>>>> +    -f fsync=10 -n 100000 -p 2 \
>>>>>>>> +    -d $SCRATCH_MNT/stress_dir`
>>>>>>>> +echo "Run fsstress $args" >>$seqres.full
>>>>>>>> +$FSSTRESS_PROG $args >/dev/null 2>&1 &
>>>>>>>> +fsstress_pid=$!
>>>>>>>> +
>>>>>>>> +echo "Start balance" >>$seqres.full
>>>>>>>> +_btrfs_stress_balance -d $SCRATCH_MNT >/dev/null 2>&1 &
>>>>>>>> +balance_pid=$!
>>>>>>>> +
>>>>>>>> +# 30s is enough to trigger bug
>>>>>>>> +sleep $((30*$TIME_FACTOR))
>>>>>>>> +kill $fsstress_pid $balance_pid
>>>>>>>> +wait
>>>>>>>> +
>>>>>>>> +# kill _btrfs_stress_balance can't end balance, so call btrfs
>>>>>>>> balance cancel
>>>>>>>> +# to cancel running or paused balance.
>>>>>>>> +$BTRFS_UTIL_PROG balance cancel $SCRATCH_MNT &> /dev/null
>>>>>>>> +
>>>>>>>> +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
>>>>>>>> +
>>>>>>>> +_scratch_unmount
>>>>>>>> +
>>>>>>>> +# generate a qgroup report and look for inconsistent groups
>>>>>>>> +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
>>>>>>>> +            grep -q -E "Counts for qgroup.*are different"
>>>>>>>> +if [ $? -ne 0 ]; then
>>>>>>>> +    echo "Silence is golden"
>>>>>>>> +    # success, all done
>>>>>>>> +    status=0
>>>>>>>> +fi
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I'm testing with 4.7-rc1 kernel and btrfs-progs v4.4, this test
>>>>>>> fails,
>>>>>>> which means btrfs check finds inconsistent groups. But according to
>>>>>>> your
>>>>>>> commit log, current kernel should pass the test. So is the failure
>>>>>>> expected?
>>>>>>>
>>>>>>> Also, just grep for different qgroup counts and print the message out
>>>>>>> if
>>>>>>> grep finds the message, so it breaks golden image on error and we
>>>>>>> know
>>>>>>> something really goes wrong. Right now test fails just because of
>>>>>>> missing "Silence is golden", which is unclear why it fails:
>>>>>>>
>>>>>>>  @@ -1,2 +1 @@
>>>>>>>   QA output created by 028
>>>>>>>  -Silence is golden
>>>>>>>
>>>>>>> Do the following instead:
>>>>>>>
>>>>>>>     $BTRFS_UTIL_PROG check ... | grep -E "..."
>>>>>>>     echo "Silence is golden"
>>>>>>>     status=0
>>>>>>>     exit
>>>>>>>
>>>>>>> And we see this on failure:
>>>>>>>
>>>>>>>  @@ -1,2 +1,3 @@
>>>>>>>   QA output created by 028
>>>>>>>  +Counts for qgroup id: 5 are different
>>>>>>>   Silence is golden
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Eryu
>>>>>>>
>>>>>>>> +
>>>>>>>> +exit
>>>>>>>> diff --git a/tests/btrfs/028.out b/tests/btrfs/028.out
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..2615f73
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/tests/btrfs/028.out
>>>>>>>> @@ -0,0 +1,2 @@
>>>>>>>> +QA output created by 028
>>>>>>>> +Silence is golden
>>>>>>>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>>>>>>>> index da0e27f..35ecf59 100644
>>>>>>>> --- a/tests/btrfs/group
>>>>>>>> +++ b/tests/btrfs/group
>>>>>>>> @@ -30,6 +30,7 @@
>>>>>>>>  025 auto quick send clone
>>>>>>>>  026 auto quick compress prealloc
>>>>>>>>  027 auto replace
>>>>>>>> +028 auto qgroup balance
>>>>>>>>  029 auto quick clone
>>>>>>>>  030 auto quick send
>>>>>>>>  031 auto quick subvol clone
>>>>>>>> --
>>>>>>>> 2.5.5
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Excuse me, I tried several versions of the kernel (4.6-rc7+ / 4.7-rc1
>>>>>> /
>>>>>> 4.7-rc3), the test didn't fail. Can you provide me with more
>>>>>> information
>>>>>> about your test environment?
>>>>>>
>>>>>>
>>>>> Sorry, this test fails after I perform it more times. It seems to be a
>>>>> bug
>>>>> of btrfs qgroup. We will continue to track this bug.
>>>>
>>>>
>>>>
>>>> Which is already known and reported some time ago:
>>>>
>>>> http://thread.gmane.org/gmane.comp.file-systems.btrfs/56687/focus=56824
>>>>
>>>> It's another regression introduced in kernel 4.2 with the large
>>>> qgroups rewrite (data extents are not accounted anymore after
>>>> balance).
>>>> It's trivial to reproduce with:
>>>>
>>>>
>>>> #!/bin/bash -x
>>>>
>>>> MNT="/mnt/sdi"
>>>> DEV="/dev/sdi"
>>>>
>>>> mkfs.btrfs --nodesize=4096 -f $DEV
>>>> mount -t btrfs $DEV $MNT
>>>>
>>>> # fill metadata space, create a tree with 1 node and 2 leaves
>>>> for ((i = 1; i <= 10; i++)); do
>>>>         mkdir $MNT/d$i
>>>> done
>>>> for ((i = 1; i <= 1; i++)); do
>>>>         xfs_io -f -c "pwrite -S 0xaa 0 8K" $MNT/f$i
>>>> done
>>>>
>>>> btrfs fi sync $MNT
>>>> btrfs quota enable $MNT
>>>> btrfs quota rescan -w $MNT
>>>> btrfs qg show $MNT
>>>>
>>>> umount $MNT
>>>> btrfsck $DEV
>>>>
>>>> mount -t btrfs $DEV $MNT
>>>>
>>>> time btrfs balance start -d $MNT
>>>>
>>>> umount $MNT
>>>> btrfsck $DEV
>>>
>>>
>>> Yes, we also found this internally.
>>>
>>> And the direct cause is already located.
>>>
>>> The direct cause is, we're increasing new data extents' reference even
>>> the
>>> referencer(fs tree) is still referring to old extents.
>>>
>>> If using ftrace to trace add/run_delayed_data_ref() and
>>> qgroup_account_extent(), one should find that, new data extents'
>>> reference
>>> is increased with both data reloc tree and final referencer.(fs tree in
>>> the
>>> test script).
>>>
>>> In that timing, backref walk can't find any root referring the new data
>>> extents.
>>> The data reloc tree is the current referencer, but qgroup won't account
>>> data
>>> reloc tree.
>>> While new data extents have fs tree backref, fs tree still refers to the
>>> old
>>> extent. So backref walk won't count it either.
>>>
>>> And the commit_trans before we really merges reloc roots makes the
>>> accounting wrong.
>>>
>>> So it seems to be an existing problem before the qgroup rework.
>>
>>
>> How can it be an existing problem before the qgroup rework if this
>> example (and others more complex involving snapshots and a large
>> number of data extents) worked on 4.1 and previous kernels?
>
>
> Because in old kernels, backref walk is unreliable, especially for such
> tricky tree block swap thing.
>
> As you can see, it's very obvious that, while fs tree is still referring to
> the old extent, it's never possible to get nr_old/new_roots to anything
> other than 0 for new extent.
>
> So for old kernels, either backref walk got wrong(as it just happend), or it
> doesn't go through qgroup accounting at all.

>From a user point of view, the results are incorrect with 4.2+ and
correct with 4.1 and older kernels.
That's all that matters and that's what defines a regression.

>
> Thanks,
> Qu
>
>
>>
>>
>>
>>>
>>> The quick fix won't be hard, just re-dirty the new data extent at
>>> merge_reloc_roots() time.
>>>
>>> But I'm still investigating if we can fix it in a better method, to
>>> increase
>>> the new data extents' reference at correct time.
>>>
>>> Thanks,
>>> Qu
>>>
>>>
>>>>
>>>>
>>>> Also, please, when you send a new version of a patch, add a changelog
>>>> describing what changed between versions. See [1] for an example.
>>>> I noticed you do this for all patches you send.
>>>>
>>>> [1]
>>>>
>>>> https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Repeated_submissions
>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Lu
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."