* Re: [PATCH] ext4/046: skip test when ext4 doesn't support bs < ps with dioread_nolock
2021-01-17 15:06 ` Eryu Guan
@ 2021-01-19 3:00 ` Yang Xu
2021-01-19 3:48 ` Theodore Ts'o
2021-01-19 6:13 ` [PATCH v2] " Yang Xu
2 siblings, 0 replies; 5+ messages in thread
From: Yang Xu @ 2021-01-19 3:00 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
Hi Eryu
> On Thu, Jan 14, 2021 at 11:55:03AM +0800, Yang Xu wrote:
>> When testing arm machine, this case fails because ps> bs and kernel doesn't
>> introduced commit c8cc88163f40 ("ext4: Add support for blocksize< pagesize in dioread_nolock").
>> Only skip this case when mounting failed.
>
> Perhaps it's the kernel that should be fixed? i.e. backport the
> mentioned patch? Depends on if we treat it as a bug or a feature I
> think..
IMO, it is a feature instead of a bug.
>
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>> ---
>> tests/ext4/046 | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/ext4/046 b/tests/ext4/046
>> index 5524024e..e307cbf1 100755
>> --- a/tests/ext4/046
>> +++ b/tests/ext4/046
>> @@ -37,7 +37,8 @@ _require_xfs_io_command "falloc"
>> _require_scratch_size $((6 * 1024 * 1024)) #kB
>>
>> _scratch_mkfs>> $seqres.full 2>&1
>> -_scratch_mount "-o dioread_nolock">> $seqres.full 2>&1
>> +_try_scratch_mount "-o dioread_nolock">>$seqres.full 2>&1 || \
>> + _notrun "mount failed, ext4 doesn't support bs< ps with dioread_nolock?"
>
> At least we should check if it's really the bs< ps case before _notrun
> the test, otherwise we may fail to find dioread_nolock mount regression.
Ok. Will send v2.
>
> Thanks,
> Eryu
>
>>
>> # Get blksz
>> blksz=$(_get_file_block_size $SCRATCH_MNT)
>> --
>> 2.23.0
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ext4/046: skip test when ext4 doesn't support bs < ps with dioread_nolock
2021-01-17 15:06 ` Eryu Guan
2021-01-19 3:00 ` Yang Xu
@ 2021-01-19 3:48 ` Theodore Ts'o
2021-01-19 6:13 ` [PATCH v2] " Yang Xu
2 siblings, 0 replies; 5+ messages in thread
From: Theodore Ts'o @ 2021-01-19 3:48 UTC (permalink / raw)
To: Eryu Guan; +Cc: Yang Xu, fstests
On Sun, Jan 17, 2021 at 11:06:24PM +0800, Eryu Guan wrote:
> On Thu, Jan 14, 2021 at 11:55:03AM +0800, Yang Xu wrote:
> > When testing arm machine, this case fails because ps > bs and kernel doesn't
> > introduced commit c8cc88163f40 ("ext4: Add support for blocksize < pagesize in dioread_nolock").
> > Only skip this case when mounting failed.
>
> Perhaps it's the kernel that should be fixed? i.e. backport the
> mentioned patch? Depends on if we treat it as a bug or a feature I
> think..
The dioread_nolock was introduced about ten years, and originally it
was an optimization that only worked when page size == block size. A
little over a year ago (October 2019, in v5.5) this was relaxed so
that the dioread_nolock mount option would be accepted when the block
size < page size.
ARM's page size can be 4k, 64k, or 1MB. At least on most ARM platform
that I've worked with (which are mobile handsets, admittedly) then
page size is 4k, and the default block size for ext4 file systems
created by ext4 is 4k. You *can* ask mke2fs to create a 64k block
size, and in theory it should work, but I don't have access to any
platforms with 64k block sizes, so 64k support has a tendency to
regress. We did fix some issues the last time someone tried to use an
ext4 file system with a 64k block sizes on an Itanium system, but that
was a couple of years ago.
As far as backporting dioread_nolock support to older kernels, it
requires a large restructuring of ext4 code to support it. It's not
something that won't be acceptable for upstream LTS kernels. I'm
assuming the issue here is that Fujitsu is trying to use some RHEL or
SLES kernel on a server-class ARM system with a 64k page size?
> At least we should check if it's really the bs < ps case before _notrun
> the test, otherwise we may fail to find dioread_nolock mount regression.
I'd tend to agree, but at least for ext4 upstream development, I run
the the full set of -g auto tests with dioread_nolock. So if there a
dioread_nolock mount regression, I'd certainly notice.
- Ted
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] ext4/046: skip test when ext4 doesn't support bs < ps with dioread_nolock
2021-01-17 15:06 ` Eryu Guan
2021-01-19 3:00 ` Yang Xu
2021-01-19 3:48 ` Theodore Ts'o
@ 2021-01-19 6:13 ` Yang Xu
2 siblings, 0 replies; 5+ messages in thread
From: Yang Xu @ 2021-01-19 6:13 UTC (permalink / raw)
To: guaneryu, tytso; +Cc: fstests, Yang Xu
When testing arm machine, this case fails because ps > bs and kernel doesn't
introduced commit c8cc88163f40 ("ext4: Add support for blocksize < pagesize in dioread_nolock").
Only skip this case when kernel complains about bs!=ps error, so we can find
dioread_nolock mount regression in the future.
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
tests/ext4/046 | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tests/ext4/046 b/tests/ext4/046
index 5524024e..c6f7a051 100755
--- a/tests/ext4/046
+++ b/tests/ext4/046
@@ -31,13 +31,22 @@ _cleanup()
# remove previous $seqres.full before test
rm -f $seqres.full
+_require_check_dmesg
_supported_fs ext4
_require_scratch
_require_xfs_io_command "falloc"
_require_scratch_size $((6 * 1024 * 1024)) #kB
_scratch_mkfs >> $seqres.full 2>&1
-_scratch_mount "-o dioread_nolock" >> $seqres.full 2>&1
+if ! _try_scratch_mount "-o dioread_nolock" >> $seqres.full 2>&1; then
+ err_str="can't mount with dioread_nolock if block size != PAGE_SIZE"
+ _check_dmesg_for ${err_str}
+ if [ $? -eq 0 ]; then
+ _notrun "mount failed, ext4 doesn't support bs < ps with dioread_nolock"
+ else
+ _fail "mount failed with dioread_nolock"
+ fi
+fi
# Get blksz
blksz=$(_get_file_block_size $SCRATCH_MNT)
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread