* [PATCH] generic/456: add check for fallocate flags
@ 2017-09-11 9:25 Xiao Yang
2017-09-11 11:03 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2017-09-11 9:25 UTC (permalink / raw)
To: fstests; +Cc: Xiao Yang
On RHEL6.9GA, this case could not emulate a crash and passed due
to unsupported collapse_range and zero_range instead of no bug.
We added check for fallocate flags to avoid confusion.
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
tests/generic/456 | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/generic/456 b/tests/generic/456
index 8debd3f..b72acea 100755
--- a/tests/generic/456
+++ b/tests/generic/456
@@ -67,11 +67,16 @@ write 0x3e5ec 0x1a14 0x21446
zero_range 0x20fac 0x6d9c 0x40000 keep_size
mapwrite 0x216ad 0x274f 0x40000
EOF
-run_check $here/ltp/fsx -d --replay-ops $fsxops $SCRATCH_MNT/testfile
+touch $tmp.dupops
+run_check $here/ltp/fsx -d --replay-ops $fsxops --record-ops=$tmp.dupops $SCRATCH_MNT/testfile
_flakey_drop_and_remount
_unmount_flakey
_cleanup_flakey
+
+ops_name=$(awk '/skip/ {printf "%s ", $2}' $tmp.dupops)
+[ -n "$ops_name" ] && _notrun "fallocate does not support $ops_name"
+
_check_scratch_fs
echo "Silence is golden"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] generic/456: add check for fallocate flags
2017-09-11 9:25 [PATCH] generic/456: add check for fallocate flags Xiao Yang
@ 2017-09-11 11:03 ` Amir Goldstein
2017-09-12 2:38 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2017-09-11 11:03 UTC (permalink / raw)
To: Xiao Yang; +Cc: fstests
On Mon, Sep 11, 2017 at 12:25 PM, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
> On RHEL6.9GA, this case could not emulate a crash and passed due
> to unsupported collapse_range and zero_range instead of no bug.
>
> We added check for fallocate flags to avoid confusion.
>
I am not sure I understand the confusion.
A bug was allegedly introduced to ext4 when introducing
collapse_range and/or insert_range and this is a regression test
for this alleged regression.
In what way is it confusing that the test passes on an old kernel?
There are a lot of tests in xfstests that test for regressions that
were introduced by commit XYZ. I don't see those tests checking
that they are running on kernel > XYZ.
BTW, this test also passes on btrfs and xfs, but it does not include
_supported_fs ext4 against confusion.
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> tests/generic/456 | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/generic/456 b/tests/generic/456
> index 8debd3f..b72acea 100755
> --- a/tests/generic/456
> +++ b/tests/generic/456
> @@ -67,11 +67,16 @@ write 0x3e5ec 0x1a14 0x21446
> zero_range 0x20fac 0x6d9c 0x40000 keep_size
> mapwrite 0x216ad 0x274f 0x40000
> EOF
> -run_check $here/ltp/fsx -d --replay-ops $fsxops $SCRATCH_MNT/testfile
> +touch $tmp.dupops
> +run_check $here/ltp/fsx -d --replay-ops $fsxops --record-ops=$tmp.dupops $SCRATCH_MNT/testfile
>
> _flakey_drop_and_remount
> _unmount_flakey
> _cleanup_flakey
> +
> +ops_name=$(awk '/skip/ {printf "%s ", $2}' $tmp.dupops)
> +[ -n "$ops_name" ] && _notrun "fallocate does not support $ops_name"
> +
If you must add some check, please add
_require_xfs_io_command "fcollapse"
_require_xfs_io_command "fzero"
It is not really a must for this test and its not even really testing if fs
supports those commands, but that is de-facto standard for not
running fcollapse/fzero tests.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] generic/456: add check for fallocate flags
2017-09-11 11:03 ` Amir Goldstein
@ 2017-09-12 2:38 ` Xiao Yang
2017-09-12 4:12 ` Eryu Guan
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2017-09-12 2:38 UTC (permalink / raw)
To: Amir Goldstein; +Cc: fstests
Hi Amir,
Thanks for your comments. :-)
Could you tell me which patch has fixed the ext4 bug?
On 2017/09/11 19:03, Amir Goldstein wrote:
> On Mon, Sep 11, 2017 at 12:25 PM, Xiao Yang<yangx.jy@cn.fujitsu.com> wrote:
>> On RHEL6.9GA, this case could not emulate a crash and passed due
>> to unsupported collapse_range and zero_range instead of no bug.
>>
>> We added check for fallocate flags to avoid confusion.
>>
> I am not sure I understand the confusion.
>
> A bug was allegedly introduced to ext4 when introducing
> collapse_range and/or insert_range and this is a regression test
> for this alleged regression.
>
> In what way is it confusing that the test passes on an old kernel?
> There are a lot of tests in xfstests that test for regressions that
> were introduced by commit XYZ. I don't see those tests checking
> that they are running on kernel> XYZ.
>
> BTW, this test also passes on btrfs and xfs, but it does not include
> _supported_fs ext4 against confusion.
On an old kernel(e.g. RHEL6.9GA), the test passed and got the following
message in ext4.
----------------------------------------------------------------------
# /var/lib/xfstests/ltp/fsx -d --replay-ops /tmp/733.fsxops
--record-ops=/tmp/733.dupops /mnt/xfstests/scratch/testfile
main: filesystem does not support fallocate mode FALLOC_FL_ZERO_RANGE,
disabling!
main: filesystem does not support fallocate mode
FALLOC_FL_COLLAPSE_RANGE, disabling!
main: filesystem does not support fallocate mode FALLOC_FL_INSERT_RANGE,
disabling!
1 write 0x137dd thru 0x21445 (0xdc69 bytes)
fallocating to largest ever: 0x16ade
2 falloc from 0xb531 to 0x16ade (0xb5ad bytes)
4 write 0x3e5ec thru 0x3ffff (0x1a14 bytes)
6 mapwrite 0x216ad thru 0x23dfb (0x274f bytes)
----------------------------------------------------------------------
We skip collapse_range and zero_range operations and cannot trigger the
expected bug in ext4.
I want to distinguish between unsupported flags and no bug. Do you
think it needs to distinguish?
>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> tests/generic/456 | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/generic/456 b/tests/generic/456
>> index 8debd3f..b72acea 100755
>> --- a/tests/generic/456
>> +++ b/tests/generic/456
>> @@ -67,11 +67,16 @@ write 0x3e5ec 0x1a14 0x21446
>> zero_range 0x20fac 0x6d9c 0x40000 keep_size
>> mapwrite 0x216ad 0x274f 0x40000
>> EOF
>> -run_check $here/ltp/fsx -d --replay-ops $fsxops $SCRATCH_MNT/testfile
>> +touch $tmp.dupops
>> +run_check $here/ltp/fsx -d --replay-ops $fsxops --record-ops=$tmp.dupops $SCRATCH_MNT/testfile
>>
>> _flakey_drop_and_remount
>> _unmount_flakey
>> _cleanup_flakey
>> +
>> +ops_name=$(awk '/skip/ {printf "%s ", $2}' $tmp.dupops)
>> +[ -n "$ops_name" ]&& _notrun "fallocate does not support $ops_name"
>> +
> If you must add some check, please add
> _require_xfs_io_command "fcollapse"
> _require_xfs_io_command "fzero"
>
> It is not really a must for this test and its not even really testing if fs
> supports those commands, but that is de-facto standard for not
> running fcollapse/fzero tests.
IMO, _require_xfs_io_command only check if xfs_io command supports
collapse_range or zero_range,
and it does not mean that fallocate(2) supports collapse_range or
zero_range.
I am not sure it is necessary to add some check.
Thanks,
Xiao Yang.
> Thanks,
> Amir.
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] generic/456: add check for fallocate flags
2017-09-12 2:38 ` Xiao Yang
@ 2017-09-12 4:12 ` Eryu Guan
2017-09-12 4:46 ` Xiao Yang
0 siblings, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2017-09-12 4:12 UTC (permalink / raw)
To: Xiao Yang; +Cc: Amir Goldstein, fstests
On Tue, Sep 12, 2017 at 10:38:49AM +0800, Xiao Yang wrote:
> Hi Amir,
>
> Thanks for your comments. :-)
> Could you tell me which patch has fixed the ext4 bug?
>
> On 2017/09/11 19:03, Amir Goldstein wrote:
> > On Mon, Sep 11, 2017 at 12:25 PM, Xiao Yang<yangx.jy@cn.fujitsu.com> wrote:
> > > On RHEL6.9GA, this case could not emulate a crash and passed due
> > > to unsupported collapse_range and zero_range instead of no bug.
> > >
> > > We added check for fallocate flags to avoid confusion.
> > >
> > I am not sure I understand the confusion.
> >
> > A bug was allegedly introduced to ext4 when introducing
> > collapse_range and/or insert_range and this is a regression test
> > for this alleged regression.
> >
> > In what way is it confusing that the test passes on an old kernel?
> > There are a lot of tests in xfstests that test for regressions that
> > were introduced by commit XYZ. I don't see those tests checking
> > that they are running on kernel> XYZ.
> >
> > BTW, this test also passes on btrfs and xfs, but it does not include
> > _supported_fs ext4 against confusion.
> On an old kernel(e.g. RHEL6.9GA), the test passed and got the following
> message in ext4.
> ----------------------------------------------------------------------
> # /var/lib/xfstests/ltp/fsx -d --replay-ops /tmp/733.fsxops
> --record-ops=/tmp/733.dupops /mnt/xfstests/scratch/testfile
> main: filesystem does not support fallocate mode FALLOC_FL_ZERO_RANGE,
> disabling!
> main: filesystem does not support fallocate mode FALLOC_FL_COLLAPSE_RANGE,
> disabling!
> main: filesystem does not support fallocate mode FALLOC_FL_INSERT_RANGE,
> disabling!
> 1 write 0x137dd thru 0x21445 (0xdc69 bytes)
> fallocating to largest ever: 0x16ade
> 2 falloc from 0xb531 to 0x16ade (0xb5ad bytes)
> 4 write 0x3e5ec thru 0x3ffff (0x1a14 bytes)
> 6 mapwrite 0x216ad thru 0x23dfb (0x274f bytes)
> ----------------------------------------------------------------------
>
> We skip collapse_range and zero_range operations and cannot trigger the
> expected bug in ext4.
>
> I want to distinguish between unsupported flags and no bug. Do you think it
> needs to distinguish?
If I understand the bug correctly, it's a bug in
{collapse,zero,insert}_range implementation, if the old kernels don't
support such operations, it's fair to say the old kernels have no such
bugs. And it's no harm to run some more tests even if the underlying
filesystem doesn't support such operations, because we replayed and
tested write and mapwrite operations too.
So I think it's fine to leave the test as it is.
> >
> > > Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> > > ---
> > > tests/generic/456 | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/generic/456 b/tests/generic/456
> > > index 8debd3f..b72acea 100755
> > > --- a/tests/generic/456
> > > +++ b/tests/generic/456
> > > @@ -67,11 +67,16 @@ write 0x3e5ec 0x1a14 0x21446
> > > zero_range 0x20fac 0x6d9c 0x40000 keep_size
> > > mapwrite 0x216ad 0x274f 0x40000
> > > EOF
> > > -run_check $here/ltp/fsx -d --replay-ops $fsxops $SCRATCH_MNT/testfile
> > > +touch $tmp.dupops
> > > +run_check $here/ltp/fsx -d --replay-ops $fsxops --record-ops=$tmp.dupops $SCRATCH_MNT/testfile
> > >
> > > _flakey_drop_and_remount
> > > _unmount_flakey
> > > _cleanup_flakey
> > > +
> > > +ops_name=$(awk '/skip/ {printf "%s ", $2}' $tmp.dupops)
> > > +[ -n "$ops_name" ]&& _notrun "fallocate does not support $ops_name"
> > > +
> > If you must add some check, please add
> > _require_xfs_io_command "fcollapse"
> > _require_xfs_io_command "fzero"
> >
> > It is not really a must for this test and its not even really testing if fs
> > supports those commands, but that is de-facto standard for not
> > running fcollapse/fzero tests.
> IMO, _require_xfs_io_command only check if xfs_io command supports
> collapse_range or zero_range,
> and it does not mean that fallocate(2) supports collapse_range or
> zero_range.
>
> I am not sure it is necessary to add some check.
xfs_io commands fcollapse, fzero, finsert are actually run by
_require_xfs_io_command on a file in $TEST_DIR, so it does check if the
underlying filesystem support such operations or not, not only the
xfs_io command.
Thanks,
Eryu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] generic/456: add check for fallocate flags
2017-09-12 4:12 ` Eryu Guan
@ 2017-09-12 4:46 ` Xiao Yang
2017-09-12 5:00 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Yang @ 2017-09-12 4:46 UTC (permalink / raw)
To: Eryu Guan; +Cc: Amir Goldstein, fstests
Hi Eryu,
Thanks for your reply.
Do you know whether this bug has been fixed or not? Could you give me a
link about the fix patch?
On 2017/09/12 12:12, Eryu Guan wrote:
> On Tue, Sep 12, 2017 at 10:38:49AM +0800, Xiao Yang wrote:
>> Hi Amir,
>>
>> Thanks for your comments. :-)
>> Could you tell me which patch has fixed the ext4 bug?
>>
>> On 2017/09/11 19:03, Amir Goldstein wrote:
>>> On Mon, Sep 11, 2017 at 12:25 PM, Xiao Yang<yangx.jy@cn.fujitsu.com> wrote:
>>>> On RHEL6.9GA, this case could not emulate a crash and passed due
>>>> to unsupported collapse_range and zero_range instead of no bug.
>>>>
>>>> We added check for fallocate flags to avoid confusion.
>>>>
>>> I am not sure I understand the confusion.
>>>
>>> A bug was allegedly introduced to ext4 when introducing
>>> collapse_range and/or insert_range and this is a regression test
>>> for this alleged regression.
>>>
>>> In what way is it confusing that the test passes on an old kernel?
>>> There are a lot of tests in xfstests that test for regressions that
>>> were introduced by commit XYZ. I don't see those tests checking
>>> that they are running on kernel> XYZ.
>>>
>>> BTW, this test also passes on btrfs and xfs, but it does not include
>>> _supported_fs ext4 against confusion.
>> On an old kernel(e.g. RHEL6.9GA), the test passed and got the following
>> message in ext4.
>> ----------------------------------------------------------------------
>> # /var/lib/xfstests/ltp/fsx -d --replay-ops /tmp/733.fsxops
>> --record-ops=/tmp/733.dupops /mnt/xfstests/scratch/testfile
>> main: filesystem does not support fallocate mode FALLOC_FL_ZERO_RANGE,
>> disabling!
>> main: filesystem does not support fallocate mode FALLOC_FL_COLLAPSE_RANGE,
>> disabling!
>> main: filesystem does not support fallocate mode FALLOC_FL_INSERT_RANGE,
>> disabling!
>> 1 write 0x137dd thru 0x21445 (0xdc69 bytes)
>> fallocating to largest ever: 0x16ade
>> 2 falloc from 0xb531 to 0x16ade (0xb5ad bytes)
>> 4 write 0x3e5ec thru 0x3ffff (0x1a14 bytes)
>> 6 mapwrite 0x216ad thru 0x23dfb (0x274f bytes)
>> ----------------------------------------------------------------------
>>
>> We skip collapse_range and zero_range operations and cannot trigger the
>> expected bug in ext4.
>>
>> I want to distinguish between unsupported flags and no bug. Do you think it
>> needs to distinguish?
> If I understand the bug correctly, it's a bug in
> {collapse,zero,insert}_range implementation, if the old kernels don't
> support such operations, it's fair to say the old kernels have no such
> bugs. And it's no harm to run some more tests even if the underlying
> filesystem doesn't support such operations, because we replayed and
> tested write and mapwrite operations too.
>
> So I think it's fine to leave the test as it is.
Agreed. :-)
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>> ---
>>>> tests/generic/456 | 7 ++++++-
>>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/generic/456 b/tests/generic/456
>>>> index 8debd3f..b72acea 100755
>>>> --- a/tests/generic/456
>>>> +++ b/tests/generic/456
>>>> @@ -67,11 +67,16 @@ write 0x3e5ec 0x1a14 0x21446
>>>> zero_range 0x20fac 0x6d9c 0x40000 keep_size
>>>> mapwrite 0x216ad 0x274f 0x40000
>>>> EOF
>>>> -run_check $here/ltp/fsx -d --replay-ops $fsxops $SCRATCH_MNT/testfile
>>>> +touch $tmp.dupops
>>>> +run_check $here/ltp/fsx -d --replay-ops $fsxops --record-ops=$tmp.dupops $SCRATCH_MNT/testfile
>>>>
>>>> _flakey_drop_and_remount
>>>> _unmount_flakey
>>>> _cleanup_flakey
>>>> +
>>>> +ops_name=$(awk '/skip/ {printf "%s ", $2}' $tmp.dupops)
>>>> +[ -n "$ops_name" ]&& _notrun "fallocate does not support $ops_name"
>>>> +
>>> If you must add some check, please add
>>> _require_xfs_io_command "fcollapse"
>>> _require_xfs_io_command "fzero"
>>>
>>> It is not really a must for this test and its not even really testing if fs
>>> supports those commands, but that is de-facto standard for not
>>> running fcollapse/fzero tests.
>> IMO, _require_xfs_io_command only check if xfs_io command supports
>> collapse_range or zero_range,
>> and it does not mean that fallocate(2) supports collapse_range or
>> zero_range.
>>
>> I am not sure it is necessary to add some check.
> xfs_io commands fcollapse, fzero, finsert are actually run by
> _require_xfs_io_command on a file in $TEST_DIR, so it does check if the
> underlying filesystem support such operations or not, not only the
> xfs_io command.
OK, i got it.
Thanks,
Xiao Yang.
> Thanks,
> Eryu
> --
> 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
>
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] generic/456: add check for fallocate flags
2017-09-12 4:46 ` Xiao Yang
@ 2017-09-12 5:00 ` Amir Goldstein
0 siblings, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2017-09-12 5:00 UTC (permalink / raw)
To: Xiao Yang; +Cc: Eryu Guan, fstests
On Tue, Sep 12, 2017 at 7:46 AM, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
> Hi Eryu,
>
> Thanks for your reply.
> Do you know whether this bug has been fixed or not? Could you give me a link
> about the fix patch?
>
The bug was not fixed. It was only reported.
Amir.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-12 5:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-11 9:25 [PATCH] generic/456: add check for fallocate flags Xiao Yang
2017-09-11 11:03 ` Amir Goldstein
2017-09-12 2:38 ` Xiao Yang
2017-09-12 4:12 ` Eryu Guan
2017-09-12 4:46 ` Xiao Yang
2017-09-12 5:00 ` Amir Goldstein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.