Re: [PATCH] generic/456: add check for fallocate flags

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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@xxxxxxxxxxxxxx>   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@xxxxxxxxxxxxxx>
---
   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@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux