[PATCH] ext4: regression test for hang when processing corrupted orphaned inode list

[PATCH] ext4: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Commit c9eb13a fixed this bug:
	ext4: fix hang when processing corrupted orphaned inode list

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 tests/ext4/022     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/022.out |  1 +
 tests/ext4/group   |  1 +
 3 files changed, 57 insertions(+)
 create mode 100755 tests/ext4/022
 create mode 100644 tests/ext4/022.out

diff --git a/tests/ext4/022 b/tests/ext4/022
new file mode 100755
index 0000000..a3cc094
--- /dev/null
+++ b/tests/ext4/022
@@ -0,0 +1,55 @@
+#! /bin/bash
+# FS QA Test 022
+#
+# Regression test for commit:
+# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
+#
+#-----------------------------------------------------------------------
+# 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"
+
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+_require_scratch
+_require_debugfs
+
+_scratch_mkfs >>$seqres.full 2>&1
+debugfs -w -R "ssv last_orphan 5" $SCRATCH_DEV >>$seqres.full 2>&1
+_scratch_mount
+
+# success, all done
+status=0
+exit
diff --git a/tests/ext4/022.out b/tests/ext4/022.out
new file mode 100644
index 0000000..0266a8a
--- /dev/null
+++ b/tests/ext4/022.out
@@ -0,0 +1 @@
+QA output created by 022
diff --git a/tests/ext4/group b/tests/ext4/group
index 9e28159..3e0146b 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -23,6 +23,7 @@
 018 fuzzers
 019 fuzzers
 020 auto quick ioctl rw
+022 auto quick
 271 auto rw quick
 301 aio auto ioctl rw stress
 302 aio auto ioctl rw stress
-- 
1.8.4.2

Re: [PATCH] ext4: regression test for hang when processing corrupted orphaned inode list

[Eryu Guan]

On Mon, Jun 13, 2016 at 05:09:12PM +0800, Guangwen Feng wrote:
> Commit c9eb13a fixed this bug:
> 	ext4: fix hang when processing corrupted orphaned inode list
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  tests/ext4/022     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/022.out |  1 +
>  tests/ext4/group   |  1 +
>  3 files changed, 57 insertions(+)
>  create mode 100755 tests/ext4/022
>  create mode 100644 tests/ext4/022.out
> 
> diff --git a/tests/ext4/022 b/tests/ext4/022
> new file mode 100755
> index 0000000..a3cc094
> --- /dev/null
> +++ b/tests/ext4/022
> @@ -0,0 +1,55 @@
> +#! /bin/bash
> +# FS QA Test 022
> +#
> +# Regression test for commit:
> +# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    cd /

Please use tab not space for indention.

And I think we need "rm -f $tmp.*" in _cleanup() even if you don't take
use any $tmp.* files, because helper functions may use the tmp files
internally, we should remove them as well.

> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs ext4

It could support ext2/ext3 as well, and move the test to shared
directory, as what shared/001 does.

> +_supported_os Linux
> +_require_scratch
> +_require_debugfs

This requires debugfs pseudo filesystem, not debugfs binary. Use
_require_command "$DEBUGFS_PROG" debugfs

But you may want to define $DEBUGFS_PROG first in common/config.

> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +debugfs -w -R "ssv last_orphan 5" $SCRATCH_DEV >>$seqres.full 2>&1

Though the bug only happens when last_orphan is set to 5, I think we can
test all reserved inode numbers, 1-10, 11 is the first non-reserved
inode.

> +_scratch_mount
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/ext4/022.out b/tests/ext4/022.out
> new file mode 100644
> index 0000000..0266a8a
> --- /dev/null
> +++ b/tests/ext4/022.out
> @@ -0,0 +1 @@
> +QA output created by 022

You need a "Silence is golden" in golden image to indicate that this
test doesn't expect any output.

> diff --git a/tests/ext4/group b/tests/ext4/group
> index 9e28159..3e0146b 100644
> --- a/tests/ext4/group
> +++ b/tests/ext4/group
> @@ -23,6 +23,7 @@
>  018 fuzzers
>  019 fuzzers
>  020 auto quick ioctl rw
> +022 auto quick

If you're not using big seq number on purpose, pick the smallest
available sequence number, that's 021 in this case.

Thanks,
Eryu

>  271 auto rw quick
>  301 aio auto ioctl rw stress
>  302 aio auto ioctl rw stress
> -- 
> 1.8.4.2
> 
> 
> 
> --
> 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] ext4: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Hi!

Thanks for your review and kindly reply.
I will rewrite the patch according to your suggestion.

On 06/13/2016 05:33 PM, Eryu Guan wrote:
> On Mon, Jun 13, 2016 at 05:09:12PM +0800, Guangwen Feng wrote:
>> Commit c9eb13a fixed this bug:
>> 	ext4: fix hang when processing corrupted orphaned inode list
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>  tests/ext4/022     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/ext4/022.out |  1 +
>>  tests/ext4/group   |  1 +
>>  3 files changed, 57 insertions(+)
>>  create mode 100755 tests/ext4/022
>>  create mode 100644 tests/ext4/022.out
>>
>> diff --git a/tests/ext4/022 b/tests/ext4/022
>> new file mode 100755
>> index 0000000..a3cc094
>> --- /dev/null
>> +++ b/tests/ext4/022
>> @@ -0,0 +1,55 @@
>> +#! /bin/bash
>> +# FS QA Test 022
>> +#
>> +# Regression test for commit:
>> +# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
>> +#
>> +#-----------------------------------------------------------------------
>> +# 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"
>> +
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +    cd /
> 
> Please use tab not space for indention.
> 
> And I think we need "rm -f $tmp.*" in _cleanup() even if you don't take
> use any $tmp.* files, because helper functions may use the tmp files
> internally, we should remove them as well.
> 
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +_supported_fs ext4
> 
> It could support ext2/ext3 as well, and move the test to shared
> directory, as what shared/001 does.
> 
>> +_supported_os Linux
>> +_require_scratch
>> +_require_debugfs
> 
> This requires debugfs pseudo filesystem, not debugfs binary. Use
> _require_command "$DEBUGFS_PROG" debugfs
> 
> But you may want to define $DEBUGFS_PROG first in common/config.
> 
>> +
>> +_scratch_mkfs >>$seqres.full 2>&1
>> +debugfs -w -R "ssv last_orphan 5" $SCRATCH_DEV >>$seqres.full 2>&1
> 
> Though the bug only happens when last_orphan is set to 5, I think we can
> test all reserved inode numbers, 1-10, 11 is the first non-reserved
> inode.
> 
>> +_scratch_mount
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/ext4/022.out b/tests/ext4/022.out
>> new file mode 100644
>> index 0000000..0266a8a
>> --- /dev/null
>> +++ b/tests/ext4/022.out
>> @@ -0,0 +1 @@
>> +QA output created by 022
> 
> You need a "Silence is golden" in golden image to indicate that this
> test doesn't expect any output.
> 
>> diff --git a/tests/ext4/group b/tests/ext4/group
>> index 9e28159..3e0146b 100644
>> --- a/tests/ext4/group
>> +++ b/tests/ext4/group
>> @@ -23,6 +23,7 @@
>>  018 fuzzers
>>  019 fuzzers
>>  020 auto quick ioctl rw
>> +022 auto quick
> 
> If you're not using big seq number on purpose, pick the smallest
> available sequence number, that's 021 in this case.

I see there is a 021 in the mailing list:
fstests: ext4: regression test for fsync transaction ids initialization

Anyway, I will change it to the smallest available sequence number for now.

Best Regards,
Guangwen Feng

> 
> Thanks,
> Eryu
> 
>>  271 auto rw quick
>>  301 aio auto ioctl rw stress
>>  302 aio auto ioctl rw stress
>> -- 
>> 1.8.4.2
>>
>>
>>
>> --
>> 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] ext4: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Hi, Eryu.
Sorry, I received email failure notification that it was undeliverable to your address.
So I resend it to the mailing list.

On 06/13/2016 06:15 PM, Guangwen Feng wrote:
> Hi!
> 
> Thanks for your review and kindly reply.
> I will rewrite the patch according to your suggestion.
> 
> On 06/13/2016 05:33 PM, Eryu Guan wrote:
>> On Mon, Jun 13, 2016 at 05:09:12PM +0800, Guangwen Feng wrote:
>>> Commit c9eb13a fixed this bug:
>>> 	ext4: fix hang when processing corrupted orphaned inode list
>>>
>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>> ---
>>>  tests/ext4/022     | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/ext4/022.out |  1 +
>>>  tests/ext4/group   |  1 +
>>>  3 files changed, 57 insertions(+)
>>>  create mode 100755 tests/ext4/022
>>>  create mode 100644 tests/ext4/022.out
>>>
>>> diff --git a/tests/ext4/022 b/tests/ext4/022
>>> new file mode 100755
>>> index 0000000..a3cc094
>>> --- /dev/null
>>> +++ b/tests/ext4/022
>>> @@ -0,0 +1,55 @@
>>> +#! /bin/bash
>>> +# FS QA Test 022
>>> +#
>>> +# Regression test for commit:
>>> +# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
>>> +#
>>> +#-----------------------------------------------------------------------
>>> +# 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"
>>> +
>>> +status=1	# failure is the default!
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_cleanup()
>>> +{
>>> +    cd /
>>
>> Please use tab not space for indention.
>>
>> And I think we need "rm -f $tmp.*" in _cleanup() even if you don't take
>> use any $tmp.* files, because helper functions may use the tmp files
>> internally, we should remove them as well.
>>
>>> +}
>>> +
>>> +# get standard environment, filters and checks
>>> +. ./common/rc
>>> +
>>> +# remove previous $seqres.full before test
>>> +rm -f $seqres.full
>>> +
>>> +# real QA test starts here
>>> +_supported_fs ext4
>>
>> It could support ext2/ext3 as well, and move the test to shared
>> directory, as what shared/001 does.
>>
>>> +_supported_os Linux
>>> +_require_scratch
>>> +_require_debugfs
>>
>> This requires debugfs pseudo filesystem, not debugfs binary. Use
>> _require_command "$DEBUGFS_PROG" debugfs
>>
>> But you may want to define $DEBUGFS_PROG first in common/config.
>>
>>> +
>>> +_scratch_mkfs >>$seqres.full 2>&1
>>> +debugfs -w -R "ssv last_orphan 5" $SCRATCH_DEV >>$seqres.full 2>&1
>>
>> Though the bug only happens when last_orphan is set to 5, I think we can
>> test all reserved inode numbers, 1-10, 11 is the first non-reserved
>> inode.
>>
>>> +_scratch_mount
>>> +
>>> +# success, all done
>>> +status=0
>>> +exit
>>> diff --git a/tests/ext4/022.out b/tests/ext4/022.out
>>> new file mode 100644
>>> index 0000000..0266a8a
>>> --- /dev/null
>>> +++ b/tests/ext4/022.out
>>> @@ -0,0 +1 @@
>>> +QA output created by 022
>>
>> You need a "Silence is golden" in golden image to indicate that this
>> test doesn't expect any output.
>>
>>> diff --git a/tests/ext4/group b/tests/ext4/group
>>> index 9e28159..3e0146b 100644
>>> --- a/tests/ext4/group
>>> +++ b/tests/ext4/group
>>> @@ -23,6 +23,7 @@
>>>  018 fuzzers
>>>  019 fuzzers
>>>  020 auto quick ioctl rw
>>> +022 auto quick
>>
>> If you're not using big seq number on purpose, pick the smallest
>> available sequence number, that's 021 in this case.
> 
> I see there is a 021 in the mailing list:
> fstests: ext4: regression test for fsync transaction ids initialization
> 
> Anyway, I will change it to the smallest available sequence number for now.
> 
> Best Regards,
> Guangwen Feng
> 
>>
>> Thanks,
>> Eryu
>>
>>>  271 auto rw quick
>>>  301 aio auto ioctl rw stress
>>>  302 aio auto ioctl rw stress
>>> -- 
>>> 1.8.4.2
>>>
>>>
>>>
>>> --
>>> 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
>>
>>
>> .
>>
> 
> 
> --
> 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 v2] shared: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Commit c9eb13a fixed this bug:
	ext4: fix hang when processing corrupted orphaned inode list

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 common/config        |  1 +
 tests/shared/004     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/004.out |  2 ++
 tests/shared/group   |  1 +
 4 files changed, 66 insertions(+)
 create mode 100755 tests/shared/004
 create mode 100644 tests/shared/004.out

diff --git a/common/config b/common/config
index cacd815..c25b1ec 100644
--- a/common/config
+++ b/common/config
@@ -195,6 +195,7 @@ export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
 export LVM_PROG="`set_prog_path lvm`"
 export CHATTR_PROG="`set_prog_path chattr`"
+export DEBUGFS_PROG="`set_prog_path debugfs`"
 
 # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
 # newer systems have udevadm command but older systems like RHEL5 don't.
diff --git a/tests/shared/004 b/tests/shared/004
new file mode 100755
index 0000000..213fba2
--- /dev/null
+++ b/tests/shared/004
@@ -0,0 +1,62 @@
+#! /bin/bash
+# FS QA Test 004
+#
+# Regression test for commit:
+# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
+#
+#-----------------------------------------------------------------------
+# 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"
+
+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
+
+# real QA test starts here
+_supported_fs ext2 ext3 ext4
+_supported_os Linux
+_require_scratch
+_require_command "$DEBUGFS_PROG" debugfs
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+echo "Silence is golden"
+
+for i in {1..10}
+do
+	_scratch_mkfs >>$seqres.full 2>&1
+	debugfs -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1
+	_scratch_mount
+	_scratch_unmount
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/shared/004.out b/tests/shared/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/shared/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/shared/group b/tests/shared/group
index ba8a3f0..55bb594 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -6,6 +6,7 @@
 001 auto quick
 002 auto metadata quick
 003 auto quick
+004 auto quick
 006 auto enospc
 032 mkfs auto quick
 051 acl udf auto quick
-- 
1.8.4.2

Re: [PATCH v2] shared: regression test for hang when processing corrupted orphaned inode list

[Eryu Guan]

On Tue, Jun 14, 2016 at 10:58:59AM +0800, Guangwen Feng wrote:
> Commit c9eb13a fixed this bug:
> 	ext4: fix hang when processing corrupted orphaned inode list
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>

Looks good to me overall. Some minor issues inline below

> ---
>  common/config        |  1 +
>  tests/shared/004     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/shared/004.out |  2 ++
>  tests/shared/group   |  1 +
>  4 files changed, 66 insertions(+)
>  create mode 100755 tests/shared/004
>  create mode 100644 tests/shared/004.out
> 
> diff --git a/common/config b/common/config
> index cacd815..c25b1ec 100644
> --- a/common/config
> +++ b/common/config
> @@ -195,6 +195,7 @@ export DUMP_PROG="`set_prog_path dump`"
>  export RESTORE_PROG="`set_prog_path restore`"
>  export LVM_PROG="`set_prog_path lvm`"
>  export CHATTR_PROG="`set_prog_path chattr`"
> +export DEBUGFS_PROG="`set_prog_path debugfs`"
>  
>  # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
>  # newer systems have udevadm command but older systems like RHEL5 don't.
> diff --git a/tests/shared/004 b/tests/shared/004
> new file mode 100755
> index 0000000..213fba2
> --- /dev/null
> +++ b/tests/shared/004
> @@ -0,0 +1,62 @@
> +#! /bin/bash
> +# FS QA Test 004
> +#
> +# Regression test for commit:
> +# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
> +#
> +#-----------------------------------------------------------------------
> +# 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"
> +
> +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
> +
> +# real QA test starts here
> +_supported_fs ext2 ext3 ext4
> +_supported_os Linux
> +_require_scratch
> +_require_command "$DEBUGFS_PROG" debugfs
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +echo "Silence is golden"
> +
> +for i in {1..10}
> +do

Good to have some comments to explain why inode 1-10 are tested.

And fstests perfers the following for loop:
	for ...; do
		do something
	done

> +	_scratch_mkfs >>$seqres.full 2>&1

Use _scratch_mkfs_sized to create smaller filesystems? So we don't spend
much time creating filesystems. This reduces test time from 57s to 2s
for me when testing with 1k block size ext2/3. e.g.

	# create smaller filesystems to save test time
	_scratch_mkfs_sized $((16 * 1024 * 1024)) >>$seqres.full 2>&1

> +	debugfs -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1

You defined $DEBUGFS_PROG, use it here :)

> +	_scratch_mount
> +	_scratch_unmount

There's a new helper to do this now, _scratch_cycle_mount

Thanks,
Eryu

Re: [PATCH v2] shared: regression test for hang when processing corrupted orphaned inode list

[Eryu Guan]

On Tue, Jun 14, 2016 at 12:39:42PM +0800, Eryu Guan wrote:
> On Tue, Jun 14, 2016 at 10:58:59AM +0800, Guangwen Feng wrote:
> > Commit c9eb13a fixed this bug:
> > 	ext4: fix hang when processing corrupted orphaned inode list
> > 
> > Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> 
> Looks good to me overall. Some minor issues inline below
> 
[snip]
> 
> > +	debugfs -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1
> 
> You defined $DEBUGFS_PROG, use it here :)
> 
> > +	_scratch_mount
> > +	_scratch_unmount
> 
> There's a new helper to do this now, _scratch_cycle_mount

Sorry, the helper doesn't help here, it umount first then mount.  You're
doing mount then umount.

Thanks,
Eryu

Re: [PATCH v2] shared: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Hi, Eryu.
Thanks for your review and kindly reply!

On 06/15/2016 11:35 AM, Eryu Guan wrote:
> On Tue, Jun 14, 2016 at 12:39:42PM +0800, Eryu Guan wrote:
>> On Tue, Jun 14, 2016 at 10:58:59AM +0800, Guangwen Feng wrote:
>>> Commit c9eb13a fixed this bug:
>>> 	ext4: fix hang when processing corrupted orphaned inode list
>>>
>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>
>> Looks good to me overall. Some minor issues inline below
>>
> [snip]
>>
>>> +	debugfs -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1
>>
>> You defined $DEBUGFS_PROG, use it here :)
>>
>>> +	_scratch_mount
>>> +	_scratch_unmount
>>
>> There's a new helper to do this now, _scratch_cycle_mount
> 
> Sorry, the helper doesn't help here, it umount first then mount.  You're
> doing mount then umount.

Got it.
I was just writing to explain this actually. :)
Thanks again.

> 
> Thanks,
> Eryu
> 
> 
> 

[PATCH v3] shared: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Commit c9eb13a fixed this bug:
	ext4: fix hang when processing corrupted orphaned inode list

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 common/config        |  1 +
 tests/shared/004     | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/004.out |  2 ++
 tests/shared/group   |  1 +
 4 files changed, 68 insertions(+)
 create mode 100755 tests/shared/004
 create mode 100644 tests/shared/004.out

diff --git a/common/config b/common/config
index cacd815..c25b1ec 100644
--- a/common/config
+++ b/common/config
@@ -195,6 +195,7 @@ export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
 export LVM_PROG="`set_prog_path lvm`"
 export CHATTR_PROG="`set_prog_path chattr`"
+export DEBUGFS_PROG="`set_prog_path debugfs`"
 
 # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
 # newer systems have udevadm command but older systems like RHEL5 don't.
diff --git a/tests/shared/004 b/tests/shared/004
new file mode 100755
index 0000000..f108910
--- /dev/null
+++ b/tests/shared/004
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test 004
+#
+# Regression test for commit:
+# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
+#
+#-----------------------------------------------------------------------
+# 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"
+
+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
+
+# real QA test starts here
+_supported_fs ext2 ext3 ext4
+_supported_os Linux
+_require_scratch
+_require_command "$DEBUGFS_PROG" debugfs
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+echo "Silence is golden"
+
+# although The bug only happens when last_orphan is set to 5
+# it is better to test all reserved inode numbers 1-10 here
+for i in {1..10}; do
+	# create smaller filesystems to save test time
+	_scratch_mkfs_sized $((16 * 1024 * 1024)) >>$seqres.full 2>&1
+	$DEBUGFS_PROG -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1
+	_scratch_mount
+	_scratch_unmount
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/shared/004.out b/tests/shared/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/shared/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/shared/group b/tests/shared/group
index ba8a3f0..55bb594 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -6,6 +6,7 @@
 001 auto quick
 002 auto metadata quick
 003 auto quick
+004 auto quick
 006 auto enospc
 032 mkfs auto quick
 051 acl udf auto quick
-- 
1.8.4.2

Re: [PATCH v3] shared: regression test for hang when processing corrupted orphaned inode list

[Dave Chinner]

On Wed, Jun 15, 2016 at 01:21:31PM +0800, Guangwen Feng wrote:
> Commit c9eb13a fixed this bug:
> 	ext4: fix hang when processing corrupted orphaned inode list

Unfortunately, I don't know what this bug is, or what the test is
actually doing from this description or from reading the actual
test.  Please assume that not just the reviewer but someone reading
this description 5 yers from now knows *nothing* about the problem
the test addresses. Hence the patch description needs to explain
more fully what the problem is that the test is exercising....

It's also unfortunate that kernel tree commits are being referenced
without saying what tree they belong to. i.e. this should read
"kernel commit ab1234cd" so we know that commit is not something
from the fstests tree, or e2fsprogs, or some other unknown tree.
Again - think about someone reading this in 5 years time and trying
to work out what it means...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH v4] shared: regression test for hang when processing corrupted orphaned inode list

[Guangwen Feng]

Kernel commit c9eb13a fixed this bug:
	ext4: fix hang when processing corrupted orphaned inode list

Bug description:
	If the orphaned inode list contains inode #5, ext4_iget()
	returns a bad inode (since the bootloader inode should never
	be referenced directly). Because of the bad inode, we end up
	processing the inode repeatedly and this hangs the machine.

Test description:
	We manually set the superblock field last_orphan to 1~10
	(although the bug only happens when last_orphan is set to 5
	it is better to test all reserved inode numbers 1~10 here)
	and mount the fs in a loop to trigger the bug.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 common/config        |  1 +
 tests/shared/004     | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/shared/004.out |  2 ++
 tests/shared/group   |  1 +
 4 files changed, 68 insertions(+)
 create mode 100755 tests/shared/004
 create mode 100644 tests/shared/004.out

diff --git a/common/config b/common/config
index cacd815..c25b1ec 100644
--- a/common/config
+++ b/common/config
@@ -195,6 +195,7 @@ export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
 export LVM_PROG="`set_prog_path lvm`"
 export CHATTR_PROG="`set_prog_path chattr`"
+export DEBUGFS_PROG="`set_prog_path debugfs`"
 
 # use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
 # newer systems have udevadm command but older systems like RHEL5 don't.
diff --git a/tests/shared/004 b/tests/shared/004
new file mode 100755
index 0000000..b9eaa15
--- /dev/null
+++ b/tests/shared/004
@@ -0,0 +1,64 @@
+#! /bin/bash
+# FS QA Test 004
+#
+# Regression test for commit:
+# c9eb13a ext4: fix hang when processing corrupted orphaned inode list
+#
+#-----------------------------------------------------------------------
+# 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"
+
+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
+
+# real QA test starts here
+_supported_fs ext2 ext3 ext4
+_supported_os Linux
+_require_scratch
+_require_command "$DEBUGFS_PROG" debugfs
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+echo "Silence is golden"
+
+# although the bug only happens when last_orphan is set to 5
+# it is better to test all reserved inode numbers 1-10 here
+for i in {1..10}; do
+	# create smaller filesystems to save test time
+	_scratch_mkfs_sized $((16 * 1024 * 1024)) >>$seqres.full 2>&1
+	$DEBUGFS_PROG -w -R "ssv last_orphan $i" $SCRATCH_DEV >>$seqres.full 2>&1
+	_scratch_mount
+	_scratch_unmount
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/shared/004.out b/tests/shared/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/shared/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/shared/group b/tests/shared/group
index ba8a3f0..55bb594 100644
--- a/tests/shared/group
+++ b/tests/shared/group
@@ -6,6 +6,7 @@
 001 auto quick
 002 auto metadata quick
 003 auto quick
+004 auto quick
 006 auto enospc
 032 mkfs auto quick
 051 acl udf auto quick
-- 
1.8.4.2