* [PATCH 1/8] common/rc: fix _check_xfs_scrub_does_unicode on newer versions of libc-bin
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-11 21:50 ` [PATCH 2/8] common/rc: fix unicode checker detection in xfs_scrub Darrick J. Wong
` (7 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: Theodore Ts'o, linux-xfs, fstests, guan
From: Theodore Ts'o <tytso@mit.edu>
Debian 10 uses ldd from glibc 2.28, where as Debian 11 uses ldd from
glibc 2.31. Sometime between glibc 2.28 and 2.31, ldd has been
changed so that the message "not a dynamic executable" is sent stderr,
where before it was sent to stdout. As a result, it caused
regressions for tests such as generic/453 which uses
_check_xfs_scurb_does_unicode:
generic/453 5s ... [22:42:03] [22:42:08]- output mismatch (see /results/xfs/results-4k/generic/453.out.bad)
--- tests/generic/453.out 2022-01-08 15:15:15.000000000 -0500
+++ /results/xfs/results-4k/generic/453.out.bad 2022-01-08 22:42:08.596982251 -0500
@@ -4,3 +4,4 @@
Test files
Uniqueness of inodes?
Test XFS online scrub, if applicable
+ not a dynamic executable
...
Fix this by sending stderr from ldd to /dev/null. This is not a
perfect solution, since it means that even if xfs_scrub was compiled
with libicui18n, we will skip the online scrub portion of generic/453.
However, this fixes the regression when runtime OS is changed from
Debian Buster to Debian Bullseye when xfsprogs is built statically.
In the long run, it would be nice if we could determine whether
xfs_scrub has unicode support without using ldd --- perhaps by
signally this in the output of xfs_scrub -V --- but we'll need to
discuss this with the xfsprogs maintainers.
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/rc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index c087875a..c63add75 100644
--- a/common/rc
+++ b/common/rc
@@ -4728,7 +4728,7 @@ _check_xfs_scrub_does_unicode() {
# We only care if xfs_scrub has unicode string support...
if ! type ldd > /dev/null 2>&1 || \
- ! ldd "${XFS_SCRUB_PROG}" | grep -q libicui18n; then
+ ! ldd "${XFS_SCRUB_PROG}" 2> /dev/null | grep -q libicui18n; then
return 1
fi
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 2/8] common/rc: fix unicode checker detection in xfs_scrub
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
2022-01-11 21:50 ` [PATCH 1/8] common/rc: fix _check_xfs_scrub_does_unicode on newer versions of libc-bin Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-11 21:50 ` [PATCH 3/8] xfs/220: fix quotarm syscall test Darrick J. Wong
` (6 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: Theodore Ts'o, linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
_check_xfs_scrub_does_unicode is still less than adequate -- if running
ldd to report the xfs_scrub binary's dynamic library dependencies
doesn't work, we could still detect support by grepping for strings that
only appear when the unicode checker is built.
Note that this isn't the final word on all of this; I will make this
easier to discover in a future xfs_scrub release.
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/rc | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/common/rc b/common/rc
index c63add75..b3289de9 100644
--- a/common/rc
+++ b/common/rc
@@ -4726,13 +4726,22 @@ _check_xfs_scrub_does_unicode() {
_supports_xfs_scrub "${mount}" "${dev}" || return 1
- # We only care if xfs_scrub has unicode string support...
- if ! type ldd > /dev/null 2>&1 || \
- ! ldd "${XFS_SCRUB_PROG}" 2> /dev/null | grep -q libicui18n; then
- return 1
+ # If the xfs_scrub binary contains the string "Unicode name.*%s", then
+ # we know that it has the ability to complain about improper Unicode
+ # names.
+ if strings "${XFS_SCRUB_PROG}" | grep -q 'Unicode name.*%s'; then
+ return 0
fi
- return 0
+ # If the xfs_scrub binary is linked against the libicui18n Unicode
+ # library, then we surmise that it contains the Unicode name checker.
+ if type ldd > /dev/null 2>&1 && \
+ ldd "${XFS_SCRUB_PROG}" 2> /dev/null | grep -q libicui18n; then
+ return 0
+ fi
+
+ # We could not establish that xfs_scrub supports unicode names.
+ return 1
}
# exfat timestamps start at 1980 and cannot be prior to epoch
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 3/8] xfs/220: fix quotarm syscall test
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
2022-01-11 21:50 ` [PATCH 1/8] common/rc: fix _check_xfs_scrub_does_unicode on newer versions of libc-bin Darrick J. Wong
2022-01-11 21:50 ` [PATCH 2/8] common/rc: fix unicode checker detection in xfs_scrub Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-12 1:14 ` xuyang2018.jy
2022-01-11 21:50 ` [PATCH 4/8] xfs: test fixes for new 5.17 behaviors Darrick J. Wong
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: xuyang2018.jy, linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
In commit 6ba125c9, we tried to adjust this fstest to deal with the
removal of the ability to turn off quota accounting via the Q_XQUOTAOFF
system call.
Unfortunately, the changes made to this test make it nonfunctional on
those newer kernels, since the Q_XQUOTARM command returns EINVAL if
quota accounting is turned on, and the changes filter out the EINVAL
error string.
Doing this wasn't /incorrect/, because, very narrowly speaking, the
intent of this test is to guard against Q_XQUOTARM returning ENOSYS when
quota has been enabled. However, this also means that we no longer test
Q_XQUOTARM's ability to truncate the quota files at all.
So, fix this test to deal with the loss of quotaoff in the same way that
the others do -- if accounting is still enabled after the 'off' command,
cycle the mount so that Q_XQUOTARM actually truncates the files.
While we're at it, enhance the test to check that XQUOTARM actually
truncated the quota files.
Fixes: 6ba125c9 ("xfs/220: avoid failure when disabling quota accounting is not supported")
Cc: xuyang2018.jy@fujitsu.com
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
tests/xfs/220 | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/tests/xfs/220 b/tests/xfs/220
index 241a7abd..88eedf51 100755
--- a/tests/xfs/220
+++ b/tests/xfs/220
@@ -52,14 +52,30 @@ _scratch_mkfs_xfs >/dev/null 2>&1
# mount with quotas enabled
_scratch_mount -o uquota
-# turn off quota and remove space allocated to the quota files
+# turn off quota accounting...
+$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV
+
+# ...but if the kernel doesn't support turning off accounting, remount with
+# noquota option to turn it off...
+if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
+ _scratch_unmount
+ _scratch_mount -o noquota
+fi
+
+before_freesp=$(_get_available_space $SCRATCH_MNT)
+
+# ...and remove space allocated to the quota files
# (this used to give wrong ENOSYS returns in 2.6.31)
-#
-# The sed expression below replaces a notrun to cater for kernels that have
-# removed the ability to disable quota accounting at runtime. On those
-# kernel this test is rather useless, and in a few years we can drop it.
-$XFS_QUOTA_PROG -x -c off -c remove $SCRATCH_DEV 2>&1 | \
- sed -e '/XFS_QUOTARM: Invalid argument/d'
+$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV
+
+# Make sure we actually freed the space used by dquot 0
+after_freesp=$(_get_available_space $SCRATCH_MNT)
+delta=$((after_freesp - before_freesp))
+
+echo "freesp $before_freesp -> $after_freesp ($delta)" >> $seqres.full
+if [ $before_freesp -ge $after_freesp ]; then
+ echo "expected Q_XQUOTARM to free space"
+fi
# and unmount again
_scratch_unmount
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/8] xfs/220: fix quotarm syscall test
2022-01-11 21:50 ` [PATCH 3/8] xfs/220: fix quotarm syscall test Darrick J. Wong
@ 2022-01-12 1:14 ` xuyang2018.jy
2022-01-14 3:23 ` xuyang2018.jy
0 siblings, 1 reply; 23+ messages in thread
From: xuyang2018.jy @ 2022-01-12 1:14 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
on 2022/1/12 5:50, Darrick J. Wong wrote:
> From: Darrick J. Wong<djwong@kernel.org>
>
> In commit 6ba125c9, we tried to adjust this fstest to deal with the
> removal of the ability to turn off quota accounting via the Q_XQUOTAOFF
> system call.
>
> Unfortunately, the changes made to this test make it nonfunctional on
> those newer kernels, since the Q_XQUOTARM command returns EINVAL if
> quota accounting is turned on, and the changes filter out the EINVAL
> error string.
>
> Doing this wasn't /incorrect/, because, very narrowly speaking, the
> intent of this test is to guard against Q_XQUOTARM returning ENOSYS when
> quota has been enabled. However, this also means that we no longer test
> Q_XQUOTARM's ability to truncate the quota files at all.
>
> So, fix this test to deal with the loss of quotaoff in the same way that
> the others do -- if accounting is still enabled after the 'off' command,
> cycle the mount so that Q_XQUOTARM actually truncates the files.
>
> While we're at it, enhance the test to check that XQUOTARM actually
> truncated the quota files.
Looks good to me,
Reviewed-by：Yang Xu <xuyang2018.jy@fujitsu.com>
Best Regards
Yang Xu
>
> Fixes: 6ba125c9 ("xfs/220: avoid failure when disabling quota accounting is not supported")
> Cc: xuyang2018.jy@fujitsu.com
> Signed-off-by: Darrick J. Wong<djwong@kernel.org>
> ---
> tests/xfs/220 | 30 +++++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
>
> diff --git a/tests/xfs/220 b/tests/xfs/220
> index 241a7abd..88eedf51 100755
> --- a/tests/xfs/220
> +++ b/tests/xfs/220
> @@ -52,14 +52,30 @@ _scratch_mkfs_xfs>/dev/null 2>&1
> # mount with quotas enabled
> _scratch_mount -o uquota
>
> -# turn off quota and remove space allocated to the quota files
> +# turn off quota accounting...
> +$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV
> +
> +# ...but if the kernel doesn't support turning off accounting, remount with
> +# noquota option to turn it off...
> +if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
> + _scratch_unmount
> + _scratch_mount -o noquota
> +fi
> +
> +before_freesp=$(_get_available_space $SCRATCH_MNT)
> +
> +# ...and remove space allocated to the quota files
> # (this used to give wrong ENOSYS returns in 2.6.31)
> -#
> -# The sed expression below replaces a notrun to cater for kernels that have
> -# removed the ability to disable quota accounting at runtime. On those
> -# kernel this test is rather useless, and in a few years we can drop it.
> -$XFS_QUOTA_PROG -x -c off -c remove $SCRATCH_DEV 2>&1 | \
> - sed -e '/XFS_QUOTARM: Invalid argument/d'
> +$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV
> +
> +# Make sure we actually freed the space used by dquot 0
> +after_freesp=$(_get_available_space $SCRATCH_MNT)
> +delta=$((after_freesp - before_freesp))
> +
> +echo "freesp $before_freesp -> $after_freesp ($delta)">> $seqres.full
> +if [ $before_freesp -ge $after_freesp ]; then
> + echo "expected Q_XQUOTARM to free space"
> +fi
>
> # and unmount again
> _scratch_unmount
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/8] xfs/220: fix quotarm syscall test
2022-01-12 1:14 ` xuyang2018.jy
@ 2022-01-14 3:23 ` xuyang2018.jy
2022-01-14 17:08 ` Darrick J. Wong
0 siblings, 1 reply; 23+ messages in thread
From: xuyang2018.jy @ 2022-01-14 3:23 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
on 2022/1/12 9:14, xuyang2018.jy@fujitsu.com wrote:
> on 2022/1/12 5:50, Darrick J. Wong wrote:
>> From: Darrick J. Wong<djwong@kernel.org>
>>
>> In commit 6ba125c9, we tried to adjust this fstest to deal with the
>> removal of the ability to turn off quota accounting via the Q_XQUOTAOFF
>> system call.
>>
>> Unfortunately, the changes made to this test make it nonfunctional on
>> those newer kernels, since the Q_XQUOTARM command returns EINVAL if
>> quota accounting is turned on, and the changes filter out the EINVAL
>> error string.
>>
>> Doing this wasn't /incorrect/, because, very narrowly speaking, the
>> intent of this test is to guard against Q_XQUOTARM returning ENOSYS when
>> quota has been enabled. However, this also means that we no longer test
>> Q_XQUOTARM's ability to truncate the quota files at all.
>>
>> So, fix this test to deal with the loss of quotaoff in the same way that
>> the others do -- if accounting is still enabled after the 'off' command,
>> cycle the mount so that Q_XQUOTARM actually truncates the files.
>>
>> While we're at it, enhance the test to check that XQUOTARM actually
>> truncated the quota files.
> Looks good to me,
> Reviewed-by：Yang Xu<xuyang2018.jy@fujitsu.com>
>
> Best Regards
> Yang Xu
>
>>
>> Fixes: 6ba125c9 ("xfs/220: avoid failure when disabling quota accounting is not supported")
>> Cc: xuyang2018.jy@fujitsu.com
>> Signed-off-by: Darrick J. Wong<djwong@kernel.org>
>> ---
>> tests/xfs/220 | 30 +++++++++++++++++++++++-------
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>
>>
>> diff --git a/tests/xfs/220 b/tests/xfs/220
>> index 241a7abd..88eedf51 100755
>> --- a/tests/xfs/220
>> +++ b/tests/xfs/220
>> @@ -52,14 +52,30 @@ _scratch_mkfs_xfs>/dev/null 2>&1
>> # mount with quotas enabled
>> _scratch_mount -o uquota
>>
>> -# turn off quota and remove space allocated to the quota files
>> +# turn off quota accounting...
>> +$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV
>> +
>> +# ...but if the kernel doesn't support turning off accounting, remount with
>> +# noquota option to turn it off...
I used MS_REMOUNT flag with mount syscall in ltp quotactl07.c, so this
is the expected behaviour?
https://patchwork.ozlabs.org/project/ltp/patch/1641973691-22981-2-git-send-email-xuyang2018.jy@fujitsu.com/
Best Regards
Yang Xu
>> +if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
>> + _scratch_unmount
>> + _scratch_mount -o noquota
>> +fi
>> +
>> +before_freesp=$(_get_available_space $SCRATCH_MNT)
>> +
>> +# ...and remove space allocated to the quota files
>> # (this used to give wrong ENOSYS returns in 2.6.31)
>> -#
>> -# The sed expression below replaces a notrun to cater for kernels that have
>> -# removed the ability to disable quota accounting at runtime. On those
>> -# kernel this test is rather useless, and in a few years we can drop it.
>> -$XFS_QUOTA_PROG -x -c off -c remove $SCRATCH_DEV 2>&1 | \
>> - sed -e '/XFS_QUOTARM: Invalid argument/d'
>> +$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV
>> +
>> +# Make sure we actually freed the space used by dquot 0
>> +after_freesp=$(_get_available_space $SCRATCH_MNT)
>> +delta=$((after_freesp - before_freesp))
>> +
>> +echo "freesp $before_freesp -> $after_freesp ($delta)">> $seqres.full
>> +if [ $before_freesp -ge $after_freesp ]; then
>> + echo "expected Q_XQUOTARM to free space"
>> +fi
>>
>> # and unmount again
>> _scratch_unmount
>>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/8] xfs/220: fix quotarm syscall test
2022-01-14 3:23 ` xuyang2018.jy
@ 2022-01-14 17:08 ` Darrick J. Wong
2022-01-17 1:06 ` xuyang2018.jy
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-14 17:08 UTC (permalink / raw)
To: xuyang2018.jy; +Cc: guaneryu, linux-xfs, fstests, guan
On Fri, Jan 14, 2022 at 03:23:50AM +0000, xuyang2018.jy@fujitsu.com wrote:
> on 2022/1/12 9:14, xuyang2018.jy@fujitsu.com wrote:
> > on 2022/1/12 5:50, Darrick J. Wong wrote:
> >> From: Darrick J. Wong<djwong@kernel.org>
> >>
> >> In commit 6ba125c9, we tried to adjust this fstest to deal with the
> >> removal of the ability to turn off quota accounting via the Q_XQUOTAOFF
> >> system call.
> >>
> >> Unfortunately, the changes made to this test make it nonfunctional on
> >> those newer kernels, since the Q_XQUOTARM command returns EINVAL if
> >> quota accounting is turned on, and the changes filter out the EINVAL
> >> error string.
> >>
> >> Doing this wasn't /incorrect/, because, very narrowly speaking, the
> >> intent of this test is to guard against Q_XQUOTARM returning ENOSYS when
> >> quota has been enabled. However, this also means that we no longer test
> >> Q_XQUOTARM's ability to truncate the quota files at all.
> >>
> >> So, fix this test to deal with the loss of quotaoff in the same way that
> >> the others do -- if accounting is still enabled after the 'off' command,
> >> cycle the mount so that Q_XQUOTARM actually truncates the files.
> >>
> >> While we're at it, enhance the test to check that XQUOTARM actually
> >> truncated the quota files.
> > Looks good to me,
> > Reviewed-by：Yang Xu<xuyang2018.jy@fujitsu.com>
> >
> > Best Regards
> > Yang Xu
> >
> >>
> >> Fixes: 6ba125c9 ("xfs/220: avoid failure when disabling quota accounting is not supported")
> >> Cc: xuyang2018.jy@fujitsu.com
> >> Signed-off-by: Darrick J. Wong<djwong@kernel.org>
> >> ---
> >> tests/xfs/220 | 30 +++++++++++++++++++++++-------
> >> 1 file changed, 23 insertions(+), 7 deletions(-)
> >>
> >>
> >> diff --git a/tests/xfs/220 b/tests/xfs/220
> >> index 241a7abd..88eedf51 100755
> >> --- a/tests/xfs/220
> >> +++ b/tests/xfs/220
> >> @@ -52,14 +52,30 @@ _scratch_mkfs_xfs>/dev/null 2>&1
> >> # mount with quotas enabled
> >> _scratch_mount -o uquota
> >>
> >> -# turn off quota and remove space allocated to the quota files
> >> +# turn off quota accounting...
> >> +$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV
> >> +
> >> +# ...but if the kernel doesn't support turning off accounting, remount with
> >> +# noquota option to turn it off...
> I used MS_REMOUNT flag with mount syscall in ltp quotactl07.c, so this
> is the expected behaviour?
No, you have to unmount completely and mount again with '-o noquota'.
In other words, "mount -o remount,noquota" isn't sufficient to disable
accounting.
--D
> https://patchwork.ozlabs.org/project/ltp/patch/1641973691-22981-2-git-send-email-xuyang2018.jy@fujitsu.com/
>
> Best Regards
> Yang Xu
> >> +if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
> >> + _scratch_unmount
> >> + _scratch_mount -o noquota
> >> +fi
> >> +
> >> +before_freesp=$(_get_available_space $SCRATCH_MNT)
> >> +
> >> +# ...and remove space allocated to the quota files
> >> # (this used to give wrong ENOSYS returns in 2.6.31)
> >> -#
> >> -# The sed expression below replaces a notrun to cater for kernels that have
> >> -# removed the ability to disable quota accounting at runtime. On those
> >> -# kernel this test is rather useless, and in a few years we can drop it.
> >> -$XFS_QUOTA_PROG -x -c off -c remove $SCRATCH_DEV 2>&1 | \
> >> - sed -e '/XFS_QUOTARM: Invalid argument/d'
> >> +$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV
> >> +
> >> +# Make sure we actually freed the space used by dquot 0
> >> +after_freesp=$(_get_available_space $SCRATCH_MNT)
> >> +delta=$((after_freesp - before_freesp))
> >> +
> >> +echo "freesp $before_freesp -> $after_freesp ($delta)">> $seqres.full
> >> +if [ $before_freesp -ge $after_freesp ]; then
> >> + echo "expected Q_XQUOTARM to free space"
> >> +fi
> >>
> >> # and unmount again
> >> _scratch_unmount
> >>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/8] xfs/220: fix quotarm syscall test
2022-01-14 17:08 ` Darrick J. Wong
@ 2022-01-17 1:06 ` xuyang2018.jy
0 siblings, 0 replies; 23+ messages in thread
From: xuyang2018.jy @ 2022-01-17 1:06 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
on 2022/1/15 1:08, Darrick J. Wong wrote:
> On Fri, Jan 14, 2022 at 03:23:50AM +0000, xuyang2018.jy@fujitsu.com wrote:
>> on 2022/1/12 9:14, xuyang2018.jy@fujitsu.com wrote:
>>> on 2022/1/12 5:50, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong<djwong@kernel.org>
>>>>
>>>> In commit 6ba125c9, we tried to adjust this fstest to deal with the
>>>> removal of the ability to turn off quota accounting via the Q_XQUOTAOFF
>>>> system call.
>>>>
>>>> Unfortunately, the changes made to this test make it nonfunctional on
>>>> those newer kernels, since the Q_XQUOTARM command returns EINVAL if
>>>> quota accounting is turned on, and the changes filter out the EINVAL
>>>> error string.
>>>>
>>>> Doing this wasn't /incorrect/, because, very narrowly speaking, the
>>>> intent of this test is to guard against Q_XQUOTARM returning ENOSYS when
>>>> quota has been enabled. However, this also means that we no longer test
>>>> Q_XQUOTARM's ability to truncate the quota files at all.
>>>>
>>>> So, fix this test to deal with the loss of quotaoff in the same way that
>>>> the others do -- if accounting is still enabled after the 'off' command,
>>>> cycle the mount so that Q_XQUOTARM actually truncates the files.
>>>>
>>>> While we're at it, enhance the test to check that XQUOTARM actually
>>>> truncated the quota files.
>>> Looks good to me,
>>> Reviewed-by：Yang Xu<xuyang2018.jy@fujitsu.com>
>>>
>>> Best Regards
>>> Yang Xu
>>>
>>>>
>>>> Fixes: 6ba125c9 ("xfs/220: avoid failure when disabling quota accounting is not supported")
>>>> Cc: xuyang2018.jy@fujitsu.com
>>>> Signed-off-by: Darrick J. Wong<djwong@kernel.org>
>>>> ---
>>>> tests/xfs/220 | 30 +++++++++++++++++++++++-------
>>>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>>>
>>>>
>>>> diff --git a/tests/xfs/220 b/tests/xfs/220
>>>> index 241a7abd..88eedf51 100755
>>>> --- a/tests/xfs/220
>>>> +++ b/tests/xfs/220
>>>> @@ -52,14 +52,30 @@ _scratch_mkfs_xfs>/dev/null 2>&1
>>>> # mount with quotas enabled
>>>> _scratch_mount -o uquota
>>>>
>>>> -# turn off quota and remove space allocated to the quota files
>>>> +# turn off quota accounting...
>>>> +$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV
>>>> +
>>>> +# ...but if the kernel doesn't support turning off accounting, remount with
>>>> +# noquota option to turn it off...
>> I used MS_REMOUNT flag with mount syscall in ltp quotactl07.c, so this
>> is the expected behaviour?
>
> No, you have to unmount completely and mount again with '-o noquota'.
> In other words, "mount -o remount,noquota" isn't sufficient to disable
> accounting.
Thanks for your answer, I understand.
Best Regards
Yang Xu
>
> --D
>
>> https://patchwork.ozlabs.org/project/ltp/patch/1641973691-22981-2-git-send-email-xuyang2018.jy@fujitsu.com/
>>
>> Best Regards
>> Yang Xu
>>>> +if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
>>>> + _scratch_unmount
>>>> + _scratch_mount -o noquota
>>>> +fi
>>>> +
>>>> +before_freesp=$(_get_available_space $SCRATCH_MNT)
>>>> +
>>>> +# ...and remove space allocated to the quota files
>>>> # (this used to give wrong ENOSYS returns in 2.6.31)
>>>> -#
>>>> -# The sed expression below replaces a notrun to cater for kernels that have
>>>> -# removed the ability to disable quota accounting at runtime. On those
>>>> -# kernel this test is rather useless, and in a few years we can drop it.
>>>> -$XFS_QUOTA_PROG -x -c off -c remove $SCRATCH_DEV 2>&1 | \
>>>> - sed -e '/XFS_QUOTARM: Invalid argument/d'
>>>> +$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV
>>>> +
>>>> +# Make sure we actually freed the space used by dquot 0
>>>> +after_freesp=$(_get_available_space $SCRATCH_MNT)
>>>> +delta=$((after_freesp - before_freesp))
>>>> +
>>>> +echo "freesp $before_freesp -> $after_freesp ($delta)">> $seqres.full
>>>> +if [ $before_freesp -ge $after_freesp ]; then
>>>> + echo "expected Q_XQUOTARM to free space"
>>>> +fi
>>>>
>>>> # and unmount again
>>>> _scratch_unmount
>>>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/8] xfs: test fixes for new 5.17 behaviors
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (2 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 3/8] xfs/220: fix quotarm syscall test Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-11 21:50 ` [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents Darrick J. Wong
` (4 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
xfs/308 and xfs/130 are two tests that tried to mess with the refcount
btree to try to trip up the COW recovery code. Now that we've made COW
recovery only happen during log recovery, we must adjust these tests to
force a log recovery. Older kernels should be ok with this, since they
unconditionally try to recover COW on mount.
Add a helper function to unmount the filesystem with a dirty log and
convert the two tests to use it. While we're at it, remove an xfs_check
test because xfs_check refuses to run on a dirty fs, and nobody cares
about xfs_check anymore.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
common/xfs | 12 ++++++++++++
tests/xfs/130 | 6 +++++-
tests/xfs/130.out | 1 +
tests/xfs/308 | 5 +----
tests/xfs/308.out | 2 --
5 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/common/xfs b/common/xfs
index bfb1bf1e..713e9fe7 100644
--- a/common/xfs
+++ b/common/xfs
@@ -776,6 +776,18 @@ _reset_xfs_sysfs_error_handling()
done
}
+# Unmount an XFS with a dirty log
+_scratch_xfs_unmount_dirty()
+{
+ local f="$SCRATCH_MNT/.dirty_umount"
+
+ rm -f "$f"
+ echo "test" > "$f"
+ sync
+ _scratch_shutdown
+ _scratch_unmount
+}
+
# Skip if we are running an older binary without the stricter input checks.
# Make multiple checks to be sure that there is no regression on the one
# selected feature check, which would skew the result.
diff --git a/tests/xfs/130 b/tests/xfs/130
index 0eb7d9c0..9465cbb0 100755
--- a/tests/xfs/130
+++ b/tests/xfs/130
@@ -44,12 +44,16 @@ _pwrite_byte 0x62 0 $((blksz * 64)) "${SCRATCH_MNT}/file0" >> "$seqres.full"
_pwrite_byte 0x61 0 $((blksz * 64)) "${SCRATCH_MNT}/file1" >> "$seqres.full"
_cp_reflink "${SCRATCH_MNT}/file0" "${SCRATCH_MNT}/file2"
_cp_reflink "${SCRATCH_MNT}/file1" "${SCRATCH_MNT}/file3"
-umount "${SCRATCH_MNT}"
+_scratch_unmount
echo "+ check fs"
_scratch_xfs_repair -n >> "$seqres.full" 2>&1 || \
_fail "xfs_repair should not fail"
+echo "+ force log recovery"
+_scratch_mount
+_scratch_xfs_unmount_dirty
+
echo "+ corrupt image"
seq 0 $((agcount - 1)) | while read ag; do
_scratch_xfs_db -x -c "agf ${ag}" -c "agf ${ag}" -c "addr refcntroot" \
diff --git a/tests/xfs/130.out b/tests/xfs/130.out
index a0eab987..6ca21ad6 100644
--- a/tests/xfs/130.out
+++ b/tests/xfs/130.out
@@ -3,6 +3,7 @@ QA output created by 130
+ mount fs image
+ make some files
+ check fs
++ force log recovery
+ corrupt image
+ mount image
+ repair fs
diff --git a/tests/xfs/308 b/tests/xfs/308
index de5ee5c1..d0f47f50 100755
--- a/tests/xfs/308
+++ b/tests/xfs/308
@@ -23,7 +23,7 @@ echo "Format"
_scratch_mkfs > $seqres.full 2>&1
_scratch_mount >> $seqres.full
is_rmap=$($XFS_INFO_PROG $SCRATCH_MNT | grep -c "rmapbt=1")
-_scratch_unmount
+_scratch_xfs_unmount_dirty
_get_agf_data() {
field="$1"
@@ -121,9 +121,6 @@ fi
_dump_status "broken fs config" >> $seqres.full
-echo "Look for leftover warning in xfs_check"
-_scratch_xfs_check | _filter_leftover
-
echo "Look for leftover warning in xfs_repair"
_scratch_xfs_repair -n 2>&1 | _filter_leftover
diff --git a/tests/xfs/308.out b/tests/xfs/308.out
index bea1de81..383cd07e 100644
--- a/tests/xfs/308.out
+++ b/tests/xfs/308.out
@@ -4,8 +4,6 @@ We need AG1 to have a single free extent
Find our extent and old counter values
Remove the extent from the freesp btrees
Add the extent to the refcount btree
-Look for leftover warning in xfs_check
-leftover CoW extent (NR/NR) len NR
Look for leftover warning in xfs_repair
leftover CoW extent (NR/NR) len NR
Mount filesystem
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (3 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 4/8] xfs: test fixes for new 5.17 behaviors Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-16 7:12 ` Eryu Guan
2022-01-18 3:37 ` Zorro Lang
2022-01-11 21:50 ` [PATCH 6/8] fsx: add support for XFS_IOC_ALLOCSP Darrick J. Wong
` (3 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
stale disk blocks for preallocation.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
.gitignore | 1
src/Makefile | 2 -
src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/xfs/832 | 56 +++++++++++++++++++++++++
tests/xfs/832.out | 2 +
5 files changed, 177 insertions(+), 1 deletion(-)
create mode 100644 src/allocstale.c
create mode 100755 tests/xfs/832
create mode 100644 tests/xfs/832.out
diff --git a/.gitignore b/.gitignore
index 65b93307..ba0c572b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,6 +56,7 @@ tags
# src/ binaries
/src/af_unix
/src/alloc
+/src/allocstale
/src/append_reader
/src/append_writer
/src/attr_replace_test
diff --git a/src/Makefile b/src/Makefile
index 1737ed0e..111ce1d9 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
t_ofd_locks t_mmap_collision mmap-write-concurrent \
t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
- t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
+ t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/allocstale.c b/src/allocstale.c
new file mode 100644
index 00000000..6253fe4c
--- /dev/null
+++ b/src/allocstale.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Oracle. All Rights Reserved.
+ * Author: Darrick J. Wong <djwong@kernel.org>
+ *
+ * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
+ * into a file.
+ */
+#include <xfs/xfs.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+
+#ifndef XFS_IOC_ALLOCSP
+# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
+#endif
+
+int
+main(
+ int argc,
+ char *argv[])
+{
+ struct stat sb;
+ char *buf, *zeroes;
+ unsigned long i;
+ unsigned long iterations;
+ int fd, ret;
+
+ if (argc != 3) {
+ fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
+ return 1;
+ }
+
+ errno = 0;
+ iterations = strtoul(argv[2], NULL, 0);
+ if (errno) {
+ perror(argv[2]);
+ return 1;
+ }
+
+ fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd < 0) {
+ perror(argv[1]);
+ return 1;
+ }
+
+ ret = fstat(fd, &sb);
+ if (ret) {
+ perror(argv[1]);
+ return 1;
+ }
+
+ buf = malloc(sb.st_blksize);
+ if (!buf) {
+ perror("pread buffer");
+ return 1;
+ }
+
+ zeroes = calloc(1, sb.st_blksize);
+ if (!zeroes) {
+ perror("zeroes buffer");
+ return 1;
+ }
+
+ for (i = 1; i <= iterations; i++) {
+ struct xfs_flock64 arg = { };
+ ssize_t read_bytes;
+ off_t offset = sb.st_blksize * i;
+
+ /* Ensure the last block of the file is a hole... */
+ ret = ftruncate(fd, offset - 1);
+ if (ret) {
+ perror("truncate");
+ return 1;
+ }
+
+ /*
+ * ...then use ALLOCSP to allocate the last block in the file.
+ * An unpatched kernel neglects to mark the new mapping
+ * unwritten or to zero the ondisk block, so...
+ */
+ arg.l_whence = SEEK_SET;
+ arg.l_start = offset;
+ ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
+ if (ret < 0) {
+ perror("ioctl");
+ return 1;
+ }
+
+ /* ... we can read old disk contents here. */
+ read_bytes = pread(fd, buf, sb.st_blksize,
+ offset - sb.st_blksize);
+ if (read_bytes < 0) {
+ perror(argv[1]);
+ return 1;
+ }
+ if (read_bytes != sb.st_blksize) {
+ fprintf(stderr, "%s: short read of %zd bytes\n",
+ argv[1], read_bytes);
+ return 1;
+ }
+
+ if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
+ fprintf(stderr, "%s: found junk near offset %zd!\n",
+ argv[1], offset - sb.st_blksize);
+ return 2;
+ }
+ }
+
+ return 0;
+}
diff --git a/tests/xfs/832 b/tests/xfs/832
new file mode 100755
index 00000000..3820ff8c
--- /dev/null
+++ b/tests/xfs/832
@@ -0,0 +1,56 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 Oracle. All Rights Reserved.
+#
+# FS QA Test 832
+#
+# Regression test for commit:
+#
+# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
+#
+. ./common/preamble
+_begin_fstest auto quick prealloc
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs xfs
+_require_test
+_require_scratch
+
+size_mb=32
+# Write a known pattern to the disk so that we can detect stale disk blocks
+# being mapped into the file. In the test author's experience, the bug will
+# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
+# to 16MB should suffice.
+$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
+MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
+
+_scratch_mount
+
+# Force the file to be created on the data device, which we pre-initialized
+# with a known pattern. The bug exists in the generic bmap code, so the choice
+# of backing device does not matter, and ignoring the rt device gets us out of
+# needing to detect things like rt extent size.
+_xfs_force_bdev data $SCRATCH_MNT
+testfile=$SCRATCH_MNT/a
+
+# Allow the test program to expand the file to consume half the free space.
+blksz=$(_get_file_block_size $SCRATCH_MNT)
+iterations=$(( (size_mb / 2) * 1048576 / blksz))
+echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
+
+# Run reproducer program and dump file contents if we see stale data. Full
+# details are in the source for the C program, but in a nutshell we run ALLOCSP
+# one block at a time to see if it'll give us blocks full of 'X'es.
+$here/src/allocstale $testfile $iterations
+res=$?
+test $res -eq 2 && od -tx1 -Ad -c $testfile
+
+# success, all done
+echo Silence is golden
+status=0
+exit
diff --git a/tests/xfs/832.out b/tests/xfs/832.out
new file mode 100644
index 00000000..bb8a6c12
--- /dev/null
+++ b/tests/xfs/832.out
@@ -0,0 +1,2 @@
+QA output created by 832
+Silence is golden
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-11 21:50 ` [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents Darrick J. Wong
@ 2022-01-16 7:12 ` Eryu Guan
2022-01-18 3:37 ` Zorro Lang
1 sibling, 0 replies; 23+ messages in thread
From: Eryu Guan @ 2022-01-16 7:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests
On Tue, Jan 11, 2022 at 01:50:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
> stale disk blocks for preallocation.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> .gitignore | 1
> src/Makefile | 2 -
> src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/832 | 56 +++++++++++++++++++++++++
> tests/xfs/832.out | 2 +
> 5 files changed, 177 insertions(+), 1 deletion(-)
> create mode 100644 src/allocstale.c
> create mode 100755 tests/xfs/832
> create mode 100644 tests/xfs/832.out
>
>
> diff --git a/.gitignore b/.gitignore
> index 65b93307..ba0c572b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -56,6 +56,7 @@ tags
> # src/ binaries
> /src/af_unix
> /src/alloc
> +/src/allocstale
> /src/append_reader
> /src/append_writer
> /src/attr_replace_test
> diff --git a/src/Makefile b/src/Makefile
> index 1737ed0e..111ce1d9 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
> t_ofd_locks t_mmap_collision mmap-write-concurrent \
> t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> - t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
> + t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
>
> LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> diff --git a/src/allocstale.c b/src/allocstale.c
> new file mode 100644
> index 00000000..6253fe4c
> --- /dev/null
> +++ b/src/allocstale.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Oracle. All Rights Reserved.
> + * Author: Darrick J. Wong <djwong@kernel.org>
> + *
> + * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
> + * into a file.
> + */
> +#include <xfs/xfs.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#ifndef XFS_IOC_ALLOCSP
> +# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> +#endif
> +
> +int
> +main(
> + int argc,
> + char *argv[])
> +{
> + struct stat sb;
> + char *buf, *zeroes;
> + unsigned long i;
> + unsigned long iterations;
> + int fd, ret;
> +
> + if (argc != 3) {
> + fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
> + return 1;
> + }
> +
> + errno = 0;
> + iterations = strtoul(argv[2], NULL, 0);
> + if (errno) {
> + perror(argv[2]);
> + return 1;
> + }
> +
> + fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
> + if (fd < 0) {
> + perror(argv[1]);
> + return 1;
> + }
> +
> + ret = fstat(fd, &sb);
> + if (ret) {
> + perror(argv[1]);
> + return 1;
> + }
> +
> + buf = malloc(sb.st_blksize);
> + if (!buf) {
> + perror("pread buffer");
> + return 1;
> + }
> +
> + zeroes = calloc(1, sb.st_blksize);
> + if (!zeroes) {
> + perror("zeroes buffer");
> + return 1;
> + }
> +
> + for (i = 1; i <= iterations; i++) {
> + struct xfs_flock64 arg = { };
> + ssize_t read_bytes;
> + off_t offset = sb.st_blksize * i;
> +
> + /* Ensure the last block of the file is a hole... */
> + ret = ftruncate(fd, offset - 1);
> + if (ret) {
> + perror("truncate");
> + return 1;
> + }
> +
> + /*
> + * ...then use ALLOCSP to allocate the last block in the file.
> + * An unpatched kernel neglects to mark the new mapping
> + * unwritten or to zero the ondisk block, so...
> + */
> + arg.l_whence = SEEK_SET;
> + arg.l_start = offset;
> + ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
> + if (ret < 0) {
> + perror("ioctl");
> + return 1;
> + }
> +
> + /* ... we can read old disk contents here. */
> + read_bytes = pread(fd, buf, sb.st_blksize,
> + offset - sb.st_blksize);
> + if (read_bytes < 0) {
> + perror(argv[1]);
> + return 1;
> + }
> + if (read_bytes != sb.st_blksize) {
> + fprintf(stderr, "%s: short read of %zd bytes\n",
> + argv[1], read_bytes);
> + return 1;
> + }
> +
> + if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
> + fprintf(stderr, "%s: found junk near offset %zd!\n",
> + argv[1], offset - sb.st_blksize);
> + return 2;
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/tests/xfs/832 b/tests/xfs/832
> new file mode 100755
> index 00000000..3820ff8c
> --- /dev/null
> +++ b/tests/xfs/832
> @@ -0,0 +1,56 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 832
> +#
> +# Regression test for commit:
> +#
> +# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick prealloc
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_test
> +_require_scratch
Added the following require rule on commit.
_require_test_program "allocstale"
Thanks,
Eryu
> +
> +size_mb=32
> +# Write a known pattern to the disk so that we can detect stale disk blocks
> +# being mapped into the file. In the test author's experience, the bug will
> +# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
> +# to 16MB should suffice.
> +$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
> +MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
> +
> +_scratch_mount
> +
> +# Force the file to be created on the data device, which we pre-initialized
> +# with a known pattern. The bug exists in the generic bmap code, so the choice
> +# of backing device does not matter, and ignoring the rt device gets us out of
> +# needing to detect things like rt extent size.
> +_xfs_force_bdev data $SCRATCH_MNT
> +testfile=$SCRATCH_MNT/a
> +
> +# Allow the test program to expand the file to consume half the free space.
> +blksz=$(_get_file_block_size $SCRATCH_MNT)
> +iterations=$(( (size_mb / 2) * 1048576 / blksz))
> +echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
> +
> +# Run reproducer program and dump file contents if we see stale data. Full
> +# details are in the source for the C program, but in a nutshell we run ALLOCSP
> +# one block at a time to see if it'll give us blocks full of 'X'es.
> +$here/src/allocstale $testfile $iterations
> +res=$?
> +test $res -eq 2 && od -tx1 -Ad -c $testfile
> +
> +# success, all done
> +echo Silence is golden
> +status=0
> +exit
> diff --git a/tests/xfs/832.out b/tests/xfs/832.out
> new file mode 100644
> index 00000000..bb8a6c12
> --- /dev/null
> +++ b/tests/xfs/832.out
> @@ -0,0 +1,2 @@
> +QA output created by 832
> +Silence is golden
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-11 21:50 ` [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents Darrick J. Wong
2022-01-16 7:12 ` Eryu Guan
@ 2022-01-18 3:37 ` Zorro Lang
2022-01-18 18:09 ` Darrick J. Wong
1 sibling, 1 reply; 23+ messages in thread
From: Zorro Lang @ 2022-01-18 3:37 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan
On Tue, Jan 11, 2022 at 01:50:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
> stale disk blocks for preallocation.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
Hi Darrick,
This case is easily failed on some multi-striped storage[1]. The full output
as [2], the out.bad output as [3].
Thanks,
Zorro
[1]
FSTYP -- xfs (non-debug)
PLATFORM -- Linux/x86_64 xxx-xxxxx-xx 5.14.0-xxx #1 SMP PREEMPT Fri Jan 14 09:24:44 UTC 2022
MKFS_OPTIONS -- -f -m crc=1,finobt=1,rmapbt=0,reflink=1,bigtime=1,inobtcount=1 -i sparse=1 /dev/sda4
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda4 /mnt/xfstests/scratch
xfs/832 _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
(see /var/lib/xfstests/results//xfs/832.full for details)
- output mismatch (see /var/lib/xfstests/results//xfs/832.out.bad)
--- tests/xfs/832.out 2022-01-15 22:18:35.175245791 -0500
+++ /var/lib/xfstests/results//xfs/832.out.bad 2022-01-15 22:20:46.630697627 -0500
@@ -1,2 +1,3 @@
QA output created by 832
+ioctl: No space left on device
Silence is golden
...
(Run 'diff -u /var/lib/xfstests/tests/xfs/832.out /var/lib/xfstests/results//xfs/832.out.bad' to see the entire diff)
Ran: xfs/832
Failures: xfs/832
Failed 1 of 1 tests
[2]
wrote 33554432/33554432 bytes at offset 0
32 MiB, 4 ops; 0.1368 sec (233.848 MiB/sec and 29.2310 ops/sec)
meta-data=/dev/sda4 isize=512 agcount=1, agsize=4112 blks
= sectsz=512 attr=2, projid32bit=1
= crc=1 finobt=1, sparse=1, rmapbt=0
= reflink=1 bigtime=1 inobtcount=1
data = bsize=4096 blocks=4112, imaxpct=25
= sunit=16 swidth=32 blks
naming =version 2 bsize=4096 ascii-ci=0, ftype=1
log =internal log bsize=4096 blocks=1872, version=2
= sectsz=512 sunit=16 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
Setting up 4096 runs for block size 4096
_check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
*** xfs_repair -n output ***
Phase 1 - find and verify superblock...
Only one AG detected - cannot validate filesystem geometry.
Use the -o force_geometry option to proceed.
*** end xfs_repair output
...
[3]
QA output created by 832
ioctl: No space left on device
Silence is golden
> .gitignore | 1
> src/Makefile | 2 -
> src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/832 | 56 +++++++++++++++++++++++++
> tests/xfs/832.out | 2 +
> 5 files changed, 177 insertions(+), 1 deletion(-)
> create mode 100644 src/allocstale.c
> create mode 100755 tests/xfs/832
> create mode 100644 tests/xfs/832.out
>
>
> diff --git a/.gitignore b/.gitignore
> index 65b93307..ba0c572b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -56,6 +56,7 @@ tags
> # src/ binaries
> /src/af_unix
> /src/alloc
> +/src/allocstale
> /src/append_reader
> /src/append_writer
> /src/attr_replace_test
> diff --git a/src/Makefile b/src/Makefile
> index 1737ed0e..111ce1d9 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
> t_ofd_locks t_mmap_collision mmap-write-concurrent \
> t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> - t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
> + t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
>
> LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> diff --git a/src/allocstale.c b/src/allocstale.c
> new file mode 100644
> index 00000000..6253fe4c
> --- /dev/null
> +++ b/src/allocstale.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Oracle. All Rights Reserved.
> + * Author: Darrick J. Wong <djwong@kernel.org>
> + *
> + * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
> + * into a file.
> + */
> +#include <xfs/xfs.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#ifndef XFS_IOC_ALLOCSP
> +# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> +#endif
> +
> +int
> +main(
> + int argc,
> + char *argv[])
> +{
> + struct stat sb;
> + char *buf, *zeroes;
> + unsigned long i;
> + unsigned long iterations;
> + int fd, ret;
> +
> + if (argc != 3) {
> + fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
> + return 1;
> + }
> +
> + errno = 0;
> + iterations = strtoul(argv[2], NULL, 0);
> + if (errno) {
> + perror(argv[2]);
> + return 1;
> + }
> +
> + fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
> + if (fd < 0) {
> + perror(argv[1]);
> + return 1;
> + }
> +
> + ret = fstat(fd, &sb);
> + if (ret) {
> + perror(argv[1]);
> + return 1;
> + }
> +
> + buf = malloc(sb.st_blksize);
> + if (!buf) {
> + perror("pread buffer");
> + return 1;
> + }
> +
> + zeroes = calloc(1, sb.st_blksize);
> + if (!zeroes) {
> + perror("zeroes buffer");
> + return 1;
> + }
> +
> + for (i = 1; i <= iterations; i++) {
> + struct xfs_flock64 arg = { };
> + ssize_t read_bytes;
> + off_t offset = sb.st_blksize * i;
> +
> + /* Ensure the last block of the file is a hole... */
> + ret = ftruncate(fd, offset - 1);
> + if (ret) {
> + perror("truncate");
> + return 1;
> + }
> +
> + /*
> + * ...then use ALLOCSP to allocate the last block in the file.
> + * An unpatched kernel neglects to mark the new mapping
> + * unwritten or to zero the ondisk block, so...
> + */
> + arg.l_whence = SEEK_SET;
> + arg.l_start = offset;
> + ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
> + if (ret < 0) {
> + perror("ioctl");
> + return 1;
> + }
> +
> + /* ... we can read old disk contents here. */
> + read_bytes = pread(fd, buf, sb.st_blksize,
> + offset - sb.st_blksize);
> + if (read_bytes < 0) {
> + perror(argv[1]);
> + return 1;
> + }
> + if (read_bytes != sb.st_blksize) {
> + fprintf(stderr, "%s: short read of %zd bytes\n",
> + argv[1], read_bytes);
> + return 1;
> + }
> +
> + if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
> + fprintf(stderr, "%s: found junk near offset %zd!\n",
> + argv[1], offset - sb.st_blksize);
> + return 2;
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/tests/xfs/832 b/tests/xfs/832
> new file mode 100755
> index 00000000..3820ff8c
> --- /dev/null
> +++ b/tests/xfs/832
> @@ -0,0 +1,56 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 832
> +#
> +# Regression test for commit:
> +#
> +# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick prealloc
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_test
> +_require_scratch
> +
> +size_mb=32
> +# Write a known pattern to the disk so that we can detect stale disk blocks
> +# being mapped into the file. In the test author's experience, the bug will
> +# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
> +# to 16MB should suffice.
> +$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
> +MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
> +
> +_scratch_mount
> +
> +# Force the file to be created on the data device, which we pre-initialized
> +# with a known pattern. The bug exists in the generic bmap code, so the choice
> +# of backing device does not matter, and ignoring the rt device gets us out of
> +# needing to detect things like rt extent size.
> +_xfs_force_bdev data $SCRATCH_MNT
> +testfile=$SCRATCH_MNT/a
> +
> +# Allow the test program to expand the file to consume half the free space.
> +blksz=$(_get_file_block_size $SCRATCH_MNT)
> +iterations=$(( (size_mb / 2) * 1048576 / blksz))
> +echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
> +
> +# Run reproducer program and dump file contents if we see stale data. Full
> +# details are in the source for the C program, but in a nutshell we run ALLOCSP
> +# one block at a time to see if it'll give us blocks full of 'X'es.
> +$here/src/allocstale $testfile $iterations
> +res=$?
> +test $res -eq 2 && od -tx1 -Ad -c $testfile
> +
> +# success, all done
> +echo Silence is golden
> +status=0
> +exit
> diff --git a/tests/xfs/832.out b/tests/xfs/832.out
> new file mode 100644
> index 00000000..bb8a6c12
> --- /dev/null
> +++ b/tests/xfs/832.out
> @@ -0,0 +1,2 @@
> +QA output created by 832
> +Silence is golden
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-18 3:37 ` Zorro Lang
@ 2022-01-18 18:09 ` Darrick J. Wong
2022-01-19 4:07 ` Zorro Lang
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-18 18:09 UTC (permalink / raw)
To: guaneryu, linux-xfs, fstests, guan
On Tue, Jan 18, 2022 at 11:37:35AM +0800, Zorro Lang wrote:
> On Tue, Jan 11, 2022 at 01:50:35PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
> > stale disk blocks for preallocation.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
>
> Hi Darrick,
>
> This case is easily failed on some multi-striped storage[1]. The full output
> as [2], the out.bad output as [3].
>
> Thanks,
> Zorro
>
> [1]
> FSTYP -- xfs (non-debug)
> PLATFORM -- Linux/x86_64 xxx-xxxxx-xx 5.14.0-xxx #1 SMP PREEMPT Fri Jan 14 09:24:44 UTC 2022
> MKFS_OPTIONS -- -f -m crc=1,finobt=1,rmapbt=0,reflink=1,bigtime=1,inobtcount=1 -i sparse=1 /dev/sda4
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda4 /mnt/xfstests/scratch
>
> xfs/832 _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> (see /var/lib/xfstests/results//xfs/832.full for details)
> - output mismatch (see /var/lib/xfstests/results//xfs/832.out.bad)
> --- tests/xfs/832.out 2022-01-15 22:18:35.175245791 -0500
> +++ /var/lib/xfstests/results//xfs/832.out.bad 2022-01-15 22:20:46.630697627 -0500
> @@ -1,2 +1,3 @@
> QA output created by 832
> +ioctl: No space left on device
> Silence is golden
> ...
> (Run 'diff -u /var/lib/xfstests/tests/xfs/832.out /var/lib/xfstests/results//xfs/832.out.bad' to see the entire diff)
> Ran: xfs/832
> Failures: xfs/832
> Failed 1 of 1 tests
>
> [2]
> wrote 33554432/33554432 bytes at offset 0
> 32 MiB, 4 ops; 0.1368 sec (233.848 MiB/sec and 29.2310 ops/sec)
> meta-data=/dev/sda4 isize=512 agcount=1, agsize=4112 blks
Single AG filesystems aren't a supported configuration. Is sda4
actually 16MB? I'm a little surprised that this was the outcome of
"_scratch_mkfs_sized 33554432" on line 32.
--D
> = sectsz=512 attr=2, projid32bit=1
> = crc=1 finobt=1, sparse=1, rmapbt=0
> = reflink=1 bigtime=1 inobtcount=1
> data = bsize=4096 blocks=4112, imaxpct=25
> = sunit=16 swidth=32 blks
> naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> log =internal log bsize=4096 blocks=1872, version=2
> = sectsz=512 sunit=16 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
> Setting up 4096 runs for block size 4096
> _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> *** xfs_repair -n output ***
> Phase 1 - find and verify superblock...
> Only one AG detected - cannot validate filesystem geometry.
> Use the -o force_geometry option to proceed.
> *** end xfs_repair output
> ...
>
> [3]
> QA output created by 832
> ioctl: No space left on device
> Silence is golden
>
>
> > .gitignore | 1
> > src/Makefile | 2 -
> > src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/832 | 56 +++++++++++++++++++++++++
> > tests/xfs/832.out | 2 +
> > 5 files changed, 177 insertions(+), 1 deletion(-)
> > create mode 100644 src/allocstale.c
> > create mode 100755 tests/xfs/832
> > create mode 100644 tests/xfs/832.out
> >
> >
> > diff --git a/.gitignore b/.gitignore
> > index 65b93307..ba0c572b 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -56,6 +56,7 @@ tags
> > # src/ binaries
> > /src/af_unix
> > /src/alloc
> > +/src/allocstale
> > /src/append_reader
> > /src/append_writer
> > /src/attr_replace_test
> > diff --git a/src/Makefile b/src/Makefile
> > index 1737ed0e..111ce1d9 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> > t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
> > t_ofd_locks t_mmap_collision mmap-write-concurrent \
> > t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> > - t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
> > + t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
> >
> > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> > diff --git a/src/allocstale.c b/src/allocstale.c
> > new file mode 100644
> > index 00000000..6253fe4c
> > --- /dev/null
> > +++ b/src/allocstale.c
> > @@ -0,0 +1,117 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2022 Oracle. All Rights Reserved.
> > + * Author: Darrick J. Wong <djwong@kernel.org>
> > + *
> > + * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
> > + * into a file.
> > + */
> > +#include <xfs/xfs.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +
> > +#ifndef XFS_IOC_ALLOCSP
> > +# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> > +#endif
> > +
> > +int
> > +main(
> > + int argc,
> > + char *argv[])
> > +{
> > + struct stat sb;
> > + char *buf, *zeroes;
> > + unsigned long i;
> > + unsigned long iterations;
> > + int fd, ret;
> > +
> > + if (argc != 3) {
> > + fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
> > + return 1;
> > + }
> > +
> > + errno = 0;
> > + iterations = strtoul(argv[2], NULL, 0);
> > + if (errno) {
> > + perror(argv[2]);
> > + return 1;
> > + }
> > +
> > + fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
> > + if (fd < 0) {
> > + perror(argv[1]);
> > + return 1;
> > + }
> > +
> > + ret = fstat(fd, &sb);
> > + if (ret) {
> > + perror(argv[1]);
> > + return 1;
> > + }
> > +
> > + buf = malloc(sb.st_blksize);
> > + if (!buf) {
> > + perror("pread buffer");
> > + return 1;
> > + }
> > +
> > + zeroes = calloc(1, sb.st_blksize);
> > + if (!zeroes) {
> > + perror("zeroes buffer");
> > + return 1;
> > + }
> > +
> > + for (i = 1; i <= iterations; i++) {
> > + struct xfs_flock64 arg = { };
> > + ssize_t read_bytes;
> > + off_t offset = sb.st_blksize * i;
> > +
> > + /* Ensure the last block of the file is a hole... */
> > + ret = ftruncate(fd, offset - 1);
> > + if (ret) {
> > + perror("truncate");
> > + return 1;
> > + }
> > +
> > + /*
> > + * ...then use ALLOCSP to allocate the last block in the file.
> > + * An unpatched kernel neglects to mark the new mapping
> > + * unwritten or to zero the ondisk block, so...
> > + */
> > + arg.l_whence = SEEK_SET;
> > + arg.l_start = offset;
> > + ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
> > + if (ret < 0) {
> > + perror("ioctl");
> > + return 1;
> > + }
> > +
> > + /* ... we can read old disk contents here. */
> > + read_bytes = pread(fd, buf, sb.st_blksize,
> > + offset - sb.st_blksize);
> > + if (read_bytes < 0) {
> > + perror(argv[1]);
> > + return 1;
> > + }
> > + if (read_bytes != sb.st_blksize) {
> > + fprintf(stderr, "%s: short read of %zd bytes\n",
> > + argv[1], read_bytes);
> > + return 1;
> > + }
> > +
> > + if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
> > + fprintf(stderr, "%s: found junk near offset %zd!\n",
> > + argv[1], offset - sb.st_blksize);
> > + return 2;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/tests/xfs/832 b/tests/xfs/832
> > new file mode 100755
> > index 00000000..3820ff8c
> > --- /dev/null
> > +++ b/tests/xfs/832
> > @@ -0,0 +1,56 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 832
> > +#
> > +# Regression test for commit:
> > +#
> > +# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick prealloc
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs xfs
> > +_require_test
> > +_require_scratch
> > +
> > +size_mb=32
> > +# Write a known pattern to the disk so that we can detect stale disk blocks
> > +# being mapped into the file. In the test author's experience, the bug will
> > +# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
> > +# to 16MB should suffice.
> > +$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
> > +MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
> > +
> > +_scratch_mount
> > +
> > +# Force the file to be created on the data device, which we pre-initialized
> > +# with a known pattern. The bug exists in the generic bmap code, so the choice
> > +# of backing device does not matter, and ignoring the rt device gets us out of
> > +# needing to detect things like rt extent size.
> > +_xfs_force_bdev data $SCRATCH_MNT
> > +testfile=$SCRATCH_MNT/a
> > +
> > +# Allow the test program to expand the file to consume half the free space.
> > +blksz=$(_get_file_block_size $SCRATCH_MNT)
> > +iterations=$(( (size_mb / 2) * 1048576 / blksz))
> > +echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
> > +
> > +# Run reproducer program and dump file contents if we see stale data. Full
> > +# details are in the source for the C program, but in a nutshell we run ALLOCSP
> > +# one block at a time to see if it'll give us blocks full of 'X'es.
> > +$here/src/allocstale $testfile $iterations
> > +res=$?
> > +test $res -eq 2 && od -tx1 -Ad -c $testfile
> > +
> > +# success, all done
> > +echo Silence is golden
> > +status=0
> > +exit
> > diff --git a/tests/xfs/832.out b/tests/xfs/832.out
> > new file mode 100644
> > index 00000000..bb8a6c12
> > --- /dev/null
> > +++ b/tests/xfs/832.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 832
> > +Silence is golden
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-18 18:09 ` Darrick J. Wong
@ 2022-01-19 4:07 ` Zorro Lang
2022-01-19 4:52 ` Darrick J. Wong
0 siblings, 1 reply; 23+ messages in thread
From: Zorro Lang @ 2022-01-19 4:07 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, fstests
On Tue, Jan 18, 2022 at 10:09:11AM -0800, Darrick J. Wong wrote:
> On Tue, Jan 18, 2022 at 11:37:35AM +0800, Zorro Lang wrote:
> > On Tue, Jan 11, 2022 at 01:50:35PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > >
> > > Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
> > > stale disk blocks for preallocation.
> > >
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> >
> > Hi Darrick,
> >
> > This case is easily failed on some multi-striped storage[1]. The full output
> > as [2], the out.bad output as [3].
> >
> > Thanks,
> > Zorro
> >
> > [1]
> > FSTYP -- xfs (non-debug)
> > PLATFORM -- Linux/x86_64 xxx-xxxxx-xx 5.14.0-xxx #1 SMP PREEMPT Fri Jan 14 09:24:44 UTC 2022
> > MKFS_OPTIONS -- -f -m crc=1,finobt=1,rmapbt=0,reflink=1,bigtime=1,inobtcount=1 -i sparse=1 /dev/sda4
> > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda4 /mnt/xfstests/scratch
> >
> > xfs/832 _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> > (see /var/lib/xfstests/results//xfs/832.full for details)
> > - output mismatch (see /var/lib/xfstests/results//xfs/832.out.bad)
> > --- tests/xfs/832.out 2022-01-15 22:18:35.175245791 -0500
> > +++ /var/lib/xfstests/results//xfs/832.out.bad 2022-01-15 22:20:46.630697627 -0500
> > @@ -1,2 +1,3 @@
> > QA output created by 832
> > +ioctl: No space left on device
> > Silence is golden
> > ...
> > (Run 'diff -u /var/lib/xfstests/tests/xfs/832.out /var/lib/xfstests/results//xfs/832.out.bad' to see the entire diff)
> > Ran: xfs/832
> > Failures: xfs/832
> > Failed 1 of 1 tests
> >
> > [2]
> > wrote 33554432/33554432 bytes at offset 0
> > 32 MiB, 4 ops; 0.1368 sec (233.848 MiB/sec and 29.2310 ops/sec)
> > meta-data=/dev/sda4 isize=512 agcount=1, agsize=4112 blks
>
> Single AG filesystems aren't a supported configuration. Is sda4
> actually 16MB? I'm a little surprised that this was the outcome of
> "_scratch_mkfs_sized 33554432" on line 32.
Sure, I'll try to reserve that test machine, to figure out if SCRATCH_DEV
is 16MB.
BTW some other testing hit log size too small error[1], the SCRATCH_DEV
information as [2]. And it can be reproduced manually by [3].
Thanks,
Zorro
[1]
# cat results//xfs/832.out.bad
QA output created by 832
log size 4085 blocks too small, minimum size is 4608 blocks
...
[2]
# mkfs.xfs -f $SCRATCH_DEV
meta-data=/dev/mapper/rhel8_hp--xfscratch isize=512 agcount=16, agsize=8192000 blks
= sectsz=512 attr=2, projid32bit=1
= crc=1 finobt=1, sparse=1, rmapbt=0
= reflink=1 bigtime=0 inobtcount=0
data = bsize=4096 blocks=131072000, imaxpct=25
= sunit=64 swidth=64 blks
naming =version 2 bsize=4096 ascii-ci=0, ftype=1
log =internal log bsize=4096 blocks=64000, version=2
= sectsz=512 sunit=64 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
[3]
# mkfs.xfs -f -d size=33554432 /dev/mapper/rhel_hp--dl380pg8--01-xfscratch
log size 4085 blocks too small, minimum size is 4608 blocks
Usage: mkfs.xfs
/* metadata */ [-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1,
inobtcount=0|1,bigtime=0|1]
...
>
> --D
>
> > = sectsz=512 attr=2, projid32bit=1
> > = crc=1 finobt=1, sparse=1, rmapbt=0
> > = reflink=1 bigtime=1 inobtcount=1
> > data = bsize=4096 blocks=4112, imaxpct=25
> > = sunit=16 swidth=32 blks
> > naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> > log =internal log bsize=4096 blocks=1872, version=2
> > = sectsz=512 sunit=16 blks, lazy-count=1
> > realtime =none extsz=4096 blocks=0, rtextents=0
> > Setting up 4096 runs for block size 4096
> > _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> > *** xfs_repair -n output ***
> > Phase 1 - find and verify superblock...
> > Only one AG detected - cannot validate filesystem geometry.
> > Use the -o force_geometry option to proceed.
> > *** end xfs_repair output
> > ...
> >
> > [3]
> > QA output created by 832
> > ioctl: No space left on device
> > Silence is golden
> >
> >
> > > .gitignore | 1
> > > src/Makefile | 2 -
> > > src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > tests/xfs/832 | 56 +++++++++++++++++++++++++
> > > tests/xfs/832.out | 2 +
> > > 5 files changed, 177 insertions(+), 1 deletion(-)
> > > create mode 100644 src/allocstale.c
> > > create mode 100755 tests/xfs/832
> > > create mode 100644 tests/xfs/832.out
> > >
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index 65b93307..ba0c572b 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -56,6 +56,7 @@ tags
> > > # src/ binaries
> > > /src/af_unix
> > > /src/alloc
> > > +/src/allocstale
> > > /src/append_reader
> > > /src/append_writer
> > > /src/attr_replace_test
> > > diff --git a/src/Makefile b/src/Makefile
> > > index 1737ed0e..111ce1d9 100644
> > > --- a/src/Makefile
> > > +++ b/src/Makefile
> > > @@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> > > t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
> > > t_ofd_locks t_mmap_collision mmap-write-concurrent \
> > > t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> > > - t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
> > > + t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
> > >
> > > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> > > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> > > diff --git a/src/allocstale.c b/src/allocstale.c
> > > new file mode 100644
> > > index 00000000..6253fe4c
> > > --- /dev/null
> > > +++ b/src/allocstale.c
> > > @@ -0,0 +1,117 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2022 Oracle. All Rights Reserved.
> > > + * Author: Darrick J. Wong <djwong@kernel.org>
> > > + *
> > > + * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
> > > + * into a file.
> > > + */
> > > +#include <xfs/xfs.h>
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +#include <unistd.h>
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> > > +#include <fcntl.h>
> > > +#include <errno.h>
> > > +#include <unistd.h>
> > > +#include <string.h>
> > > +
> > > +#ifndef XFS_IOC_ALLOCSP
> > > +# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> > > +#endif
> > > +
> > > +int
> > > +main(
> > > + int argc,
> > > + char *argv[])
> > > +{
> > > + struct stat sb;
> > > + char *buf, *zeroes;
> > > + unsigned long i;
> > > + unsigned long iterations;
> > > + int fd, ret;
> > > +
> > > + if (argc != 3) {
> > > + fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
> > > + return 1;
> > > + }
> > > +
> > > + errno = 0;
> > > + iterations = strtoul(argv[2], NULL, 0);
> > > + if (errno) {
> > > + perror(argv[2]);
> > > + return 1;
> > > + }
> > > +
> > > + fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
> > > + if (fd < 0) {
> > > + perror(argv[1]);
> > > + return 1;
> > > + }
> > > +
> > > + ret = fstat(fd, &sb);
> > > + if (ret) {
> > > + perror(argv[1]);
> > > + return 1;
> > > + }
> > > +
> > > + buf = malloc(sb.st_blksize);
> > > + if (!buf) {
> > > + perror("pread buffer");
> > > + return 1;
> > > + }
> > > +
> > > + zeroes = calloc(1, sb.st_blksize);
> > > + if (!zeroes) {
> > > + perror("zeroes buffer");
> > > + return 1;
> > > + }
> > > +
> > > + for (i = 1; i <= iterations; i++) {
> > > + struct xfs_flock64 arg = { };
> > > + ssize_t read_bytes;
> > > + off_t offset = sb.st_blksize * i;
> > > +
> > > + /* Ensure the last block of the file is a hole... */
> > > + ret = ftruncate(fd, offset - 1);
> > > + if (ret) {
> > > + perror("truncate");
> > > + return 1;
> > > + }
> > > +
> > > + /*
> > > + * ...then use ALLOCSP to allocate the last block in the file.
> > > + * An unpatched kernel neglects to mark the new mapping
> > > + * unwritten or to zero the ondisk block, so...
> > > + */
> > > + arg.l_whence = SEEK_SET;
> > > + arg.l_start = offset;
> > > + ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
> > > + if (ret < 0) {
> > > + perror("ioctl");
> > > + return 1;
> > > + }
> > > +
> > > + /* ... we can read old disk contents here. */
> > > + read_bytes = pread(fd, buf, sb.st_blksize,
> > > + offset - sb.st_blksize);
> > > + if (read_bytes < 0) {
> > > + perror(argv[1]);
> > > + return 1;
> > > + }
> > > + if (read_bytes != sb.st_blksize) {
> > > + fprintf(stderr, "%s: short read of %zd bytes\n",
> > > + argv[1], read_bytes);
> > > + return 1;
> > > + }
> > > +
> > > + if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
> > > + fprintf(stderr, "%s: found junk near offset %zd!\n",
> > > + argv[1], offset - sb.st_blksize);
> > > + return 2;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > diff --git a/tests/xfs/832 b/tests/xfs/832
> > > new file mode 100755
> > > index 00000000..3820ff8c
> > > --- /dev/null
> > > +++ b/tests/xfs/832
> > > @@ -0,0 +1,56 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > +#
> > > +# FS QA Test 832
> > > +#
> > > +# Regression test for commit:
> > > +#
> > > +# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick prealloc
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs xfs
> > > +_require_test
> > > +_require_scratch
> > > +
> > > +size_mb=32
> > > +# Write a known pattern to the disk so that we can detect stale disk blocks
> > > +# being mapped into the file. In the test author's experience, the bug will
> > > +# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
> > > +# to 16MB should suffice.
> > > +$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
> > > +MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
> > > +
> > > +_scratch_mount
> > > +
> > > +# Force the file to be created on the data device, which we pre-initialized
> > > +# with a known pattern. The bug exists in the generic bmap code, so the choice
> > > +# of backing device does not matter, and ignoring the rt device gets us out of
> > > +# needing to detect things like rt extent size.
> > > +_xfs_force_bdev data $SCRATCH_MNT
> > > +testfile=$SCRATCH_MNT/a
> > > +
> > > +# Allow the test program to expand the file to consume half the free space.
> > > +blksz=$(_get_file_block_size $SCRATCH_MNT)
> > > +iterations=$(( (size_mb / 2) * 1048576 / blksz))
> > > +echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
> > > +
> > > +# Run reproducer program and dump file contents if we see stale data. Full
> > > +# details are in the source for the C program, but in a nutshell we run ALLOCSP
> > > +# one block at a time to see if it'll give us blocks full of 'X'es.
> > > +$here/src/allocstale $testfile $iterations
> > > +res=$?
> > > +test $res -eq 2 && od -tx1 -Ad -c $testfile
> > > +
> > > +# success, all done
> > > +echo Silence is golden
> > > +status=0
> > > +exit
> > > diff --git a/tests/xfs/832.out b/tests/xfs/832.out
> > > new file mode 100644
> > > index 00000000..bb8a6c12
> > > --- /dev/null
> > > +++ b/tests/xfs/832.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 832
> > > +Silence is golden
> > >
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents
2022-01-19 4:07 ` Zorro Lang
@ 2022-01-19 4:52 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-19 4:52 UTC (permalink / raw)
To: linux-xfs, fstests
On Wed, Jan 19, 2022 at 12:07:06PM +0800, Zorro Lang wrote:
> On Tue, Jan 18, 2022 at 10:09:11AM -0800, Darrick J. Wong wrote:
> > On Tue, Jan 18, 2022 at 11:37:35AM +0800, Zorro Lang wrote:
> > > On Tue, Jan 11, 2022 at 01:50:35PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > >
> > > > Add a regression test to check that XFS_IOC_ALLOCSP isn't handing out
> > > > stale disk blocks for preallocation.
> > > >
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > >
> > > Hi Darrick,
> > >
> > > This case is easily failed on some multi-striped storage[1]. The full output
> > > as [2], the out.bad output as [3].
> > >
> > > Thanks,
> > > Zorro
> > >
> > > [1]
> > > FSTYP -- xfs (non-debug)
> > > PLATFORM -- Linux/x86_64 xxx-xxxxx-xx 5.14.0-xxx #1 SMP PREEMPT Fri Jan 14 09:24:44 UTC 2022
> > > MKFS_OPTIONS -- -f -m crc=1,finobt=1,rmapbt=0,reflink=1,bigtime=1,inobtcount=1 -i sparse=1 /dev/sda4
> > > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda4 /mnt/xfstests/scratch
> > >
> > > xfs/832 _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> > > (see /var/lib/xfstests/results//xfs/832.full for details)
> > > - output mismatch (see /var/lib/xfstests/results//xfs/832.out.bad)
> > > --- tests/xfs/832.out 2022-01-15 22:18:35.175245791 -0500
> > > +++ /var/lib/xfstests/results//xfs/832.out.bad 2022-01-15 22:20:46.630697627 -0500
> > > @@ -1,2 +1,3 @@
> > > QA output created by 832
> > > +ioctl: No space left on device
> > > Silence is golden
> > > ...
> > > (Run 'diff -u /var/lib/xfstests/tests/xfs/832.out /var/lib/xfstests/results//xfs/832.out.bad' to see the entire diff)
> > > Ran: xfs/832
> > > Failures: xfs/832
> > > Failed 1 of 1 tests
> > >
> > > [2]
> > > wrote 33554432/33554432 bytes at offset 0
> > > 32 MiB, 4 ops; 0.1368 sec (233.848 MiB/sec and 29.2310 ops/sec)
> > > meta-data=/dev/sda4 isize=512 agcount=1, agsize=4112 blks
> >
> > Single AG filesystems aren't a supported configuration. Is sda4
> > actually 16MB? I'm a little surprised that this was the outcome of
> > "_scratch_mkfs_sized 33554432" on line 32.
>
> Sure, I'll try to reserve that test machine, to figure out if SCRATCH_DEV
> is 16MB.
>
> BTW some other testing hit log size too small error[1], the SCRATCH_DEV
> information as [2]. And it can be reproduced manually by [3].
>
> Thanks,
> Zorro
>
> [1]
> # cat results//xfs/832.out.bad
> QA output created by 832
> log size 4085 blocks too small, minimum size is 4608 blocks
> ...
>
> [2]
> # mkfs.xfs -f $SCRATCH_DEV
> meta-data=/dev/mapper/rhel8_hp--xfscratch isize=512 agcount=16, agsize=8192000 blks
> = sectsz=512 attr=2, projid32bit=1
> = crc=1 finobt=1, sparse=1, rmapbt=0
> = reflink=1 bigtime=0 inobtcount=0
> data = bsize=4096 blocks=131072000, imaxpct=25
> = sunit=64 swidth=64 blks
> naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> log =internal log bsize=4096 blocks=64000, version=2
> = sectsz=512 sunit=64 blks, lazy-count=1
> realtime =none extsz=4096 blocks=0, rtextents=0
>
> [3]
> # mkfs.xfs -f -d size=33554432 /dev/mapper/rhel_hp--dl380pg8--01-xfscratch
> log size 4085 blocks too small, minimum size is 4608 blocks
Echoing a conversation we had on IRC: I think I'll adjust this test to
write 256M to the scratch device and create a 256M filesystem to try to
avoid all these "fs too small" issues.
--D
> Usage: mkfs.xfs
> /* metadata */ [-m crc=0|1,finobt=0|1,uuid=xxx,rmapbt=0|1,reflink=0|1,
> inobtcount=0|1,bigtime=0|1]
> ...
>
> >
> > --D
> >
> > > = sectsz=512 attr=2, projid32bit=1
> > > = crc=1 finobt=1, sparse=1, rmapbt=0
> > > = reflink=1 bigtime=1 inobtcount=1
> > > data = bsize=4096 blocks=4112, imaxpct=25
> > > = sunit=16 swidth=32 blks
> > > naming =version 2 bsize=4096 ascii-ci=0, ftype=1
> > > log =internal log bsize=4096 blocks=1872, version=2
> > > = sectsz=512 sunit=16 blks, lazy-count=1
> > > realtime =none extsz=4096 blocks=0, rtextents=0
> > > Setting up 4096 runs for block size 4096
> > > _check_xfs_filesystem: filesystem on /dev/sda4 is inconsistent (r)
> > > *** xfs_repair -n output ***
> > > Phase 1 - find and verify superblock...
> > > Only one AG detected - cannot validate filesystem geometry.
> > > Use the -o force_geometry option to proceed.
> > > *** end xfs_repair output
> > > ...
> > >
> > > [3]
> > > QA output created by 832
> > > ioctl: No space left on device
> > > Silence is golden
> > >
> > >
> > > > .gitignore | 1
> > > > src/Makefile | 2 -
> > > > src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > tests/xfs/832 | 56 +++++++++++++++++++++++++
> > > > tests/xfs/832.out | 2 +
> > > > 5 files changed, 177 insertions(+), 1 deletion(-)
> > > > create mode 100644 src/allocstale.c
> > > > create mode 100755 tests/xfs/832
> > > > create mode 100644 tests/xfs/832.out
> > > >
> > > >
> > > > diff --git a/.gitignore b/.gitignore
> > > > index 65b93307..ba0c572b 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -56,6 +56,7 @@ tags
> > > > # src/ binaries
> > > > /src/af_unix
> > > > /src/alloc
> > > > +/src/allocstale
> > > > /src/append_reader
> > > > /src/append_writer
> > > > /src/attr_replace_test
> > > > diff --git a/src/Makefile b/src/Makefile
> > > > index 1737ed0e..111ce1d9 100644
> > > > --- a/src/Makefile
> > > > +++ b/src/Makefile
> > > > @@ -18,7 +18,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
> > > > t_ext4_dax_journal_corruption t_ext4_dax_inline_corruption \
> > > > t_ofd_locks t_mmap_collision mmap-write-concurrent \
> > > > t_get_file_time t_create_short_dirs t_create_long_dirs t_enospc \
> > > > - t_mmap_writev_overlap checkpoint_journal mmap-rw-fault
> > > > + t_mmap_writev_overlap checkpoint_journal mmap-rw-fault allocstale
> > > >
> > > > LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> > > > preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
> > > > diff --git a/src/allocstale.c b/src/allocstale.c
> > > > new file mode 100644
> > > > index 00000000..6253fe4c
> > > > --- /dev/null
> > > > +++ b/src/allocstale.c
> > > > @@ -0,0 +1,117 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2022 Oracle. All Rights Reserved.
> > > > + * Author: Darrick J. Wong <djwong@kernel.org>
> > > > + *
> > > > + * Test program to try to trip over XFS_IOC_ALLOCSP mapping stale disk blocks
> > > > + * into a file.
> > > > + */
> > > > +#include <xfs/xfs.h>
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <unistd.h>
> > > > +#include <sys/types.h>
> > > > +#include <sys/stat.h>
> > > > +#include <fcntl.h>
> > > > +#include <errno.h>
> > > > +#include <unistd.h>
> > > > +#include <string.h>
> > > > +
> > > > +#ifndef XFS_IOC_ALLOCSP
> > > > +# define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
> > > > +#endif
> > > > +
> > > > +int
> > > > +main(
> > > > + int argc,
> > > > + char *argv[])
> > > > +{
> > > > + struct stat sb;
> > > > + char *buf, *zeroes;
> > > > + unsigned long i;
> > > > + unsigned long iterations;
> > > > + int fd, ret;
> > > > +
> > > > + if (argc != 3) {
> > > > + fprintf(stderr, "Usage: %s filename iterations\n", argv[0]);
> > > > + return 1;
> > > > + }
> > > > +
> > > > + errno = 0;
> > > > + iterations = strtoul(argv[2], NULL, 0);
> > > > + if (errno) {
> > > > + perror(argv[2]);
> > > > + return 1;
> > > > + }
> > > > +
> > > > + fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0600);
> > > > + if (fd < 0) {
> > > > + perror(argv[1]);
> > > > + return 1;
> > > > + }
> > > > +
> > > > + ret = fstat(fd, &sb);
> > > > + if (ret) {
> > > > + perror(argv[1]);
> > > > + return 1;
> > > > + }
> > > > +
> > > > + buf = malloc(sb.st_blksize);
> > > > + if (!buf) {
> > > > + perror("pread buffer");
> > > > + return 1;
> > > > + }
> > > > +
> > > > + zeroes = calloc(1, sb.st_blksize);
> > > > + if (!zeroes) {
> > > > + perror("zeroes buffer");
> > > > + return 1;
> > > > + }
> > > > +
> > > > + for (i = 1; i <= iterations; i++) {
> > > > + struct xfs_flock64 arg = { };
> > > > + ssize_t read_bytes;
> > > > + off_t offset = sb.st_blksize * i;
> > > > +
> > > > + /* Ensure the last block of the file is a hole... */
> > > > + ret = ftruncate(fd, offset - 1);
> > > > + if (ret) {
> > > > + perror("truncate");
> > > > + return 1;
> > > > + }
> > > > +
> > > > + /*
> > > > + * ...then use ALLOCSP to allocate the last block in the file.
> > > > + * An unpatched kernel neglects to mark the new mapping
> > > > + * unwritten or to zero the ondisk block, so...
> > > > + */
> > > > + arg.l_whence = SEEK_SET;
> > > > + arg.l_start = offset;
> > > > + ret = ioctl(fd, XFS_IOC_ALLOCSP, &arg);
> > > > + if (ret < 0) {
> > > > + perror("ioctl");
> > > > + return 1;
> > > > + }
> > > > +
> > > > + /* ... we can read old disk contents here. */
> > > > + read_bytes = pread(fd, buf, sb.st_blksize,
> > > > + offset - sb.st_blksize);
> > > > + if (read_bytes < 0) {
> > > > + perror(argv[1]);
> > > > + return 1;
> > > > + }
> > > > + if (read_bytes != sb.st_blksize) {
> > > > + fprintf(stderr, "%s: short read of %zd bytes\n",
> > > > + argv[1], read_bytes);
> > > > + return 1;
> > > > + }
> > > > +
> > > > + if (memcmp(zeroes, buf, sb.st_blksize) != 0) {
> > > > + fprintf(stderr, "%s: found junk near offset %zd!\n",
> > > > + argv[1], offset - sb.st_blksize);
> > > > + return 2;
> > > > + }
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > diff --git a/tests/xfs/832 b/tests/xfs/832
> > > > new file mode 100755
> > > > index 00000000..3820ff8c
> > > > --- /dev/null
> > > > +++ b/tests/xfs/832
> > > > @@ -0,0 +1,56 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2022 Oracle. All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 832
> > > > +#
> > > > +# Regression test for commit:
> > > > +#
> > > > +# 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto quick prealloc
> > > > +
> > > > +# Import common functions.
> > > > +. ./common/filter
> > > > +
> > > > +# real QA test starts here
> > > > +
> > > > +# Modify as appropriate.
> > > > +_supported_fs xfs
> > > > +_require_test
> > > > +_require_scratch
> > > > +
> > > > +size_mb=32
> > > > +# Write a known pattern to the disk so that we can detect stale disk blocks
> > > > +# being mapped into the file. In the test author's experience, the bug will
> > > > +# reproduce within the first 500KB's worth of ALLOCSP calls, so running up
> > > > +# to 16MB should suffice.
> > > > +$XFS_IO_PROG -d -c "pwrite -S 0x58 -b 8m 0 ${size_mb}m" $SCRATCH_DEV > $seqres.full
> > > > +MKFS_OPTIONS="-K $MKFS_OPTIONS" _scratch_mkfs_sized $((size_mb * 1048576)) >> $seqres.full
> > > > +
> > > > +_scratch_mount
> > > > +
> > > > +# Force the file to be created on the data device, which we pre-initialized
> > > > +# with a known pattern. The bug exists in the generic bmap code, so the choice
> > > > +# of backing device does not matter, and ignoring the rt device gets us out of
> > > > +# needing to detect things like rt extent size.
> > > > +_xfs_force_bdev data $SCRATCH_MNT
> > > > +testfile=$SCRATCH_MNT/a
> > > > +
> > > > +# Allow the test program to expand the file to consume half the free space.
> > > > +blksz=$(_get_file_block_size $SCRATCH_MNT)
> > > > +iterations=$(( (size_mb / 2) * 1048576 / blksz))
> > > > +echo "Setting up $iterations runs for block size $blksz" >> $seqres.full
> > > > +
> > > > +# Run reproducer program and dump file contents if we see stale data. Full
> > > > +# details are in the source for the C program, but in a nutshell we run ALLOCSP
> > > > +# one block at a time to see if it'll give us blocks full of 'X'es.
> > > > +$here/src/allocstale $testfile $iterations
> > > > +res=$?
> > > > +test $res -eq 2 && od -tx1 -Ad -c $testfile
> > > > +
> > > > +# success, all done
> > > > +echo Silence is golden
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/xfs/832.out b/tests/xfs/832.out
> > > > new file mode 100644
> > > > index 00000000..bb8a6c12
> > > > --- /dev/null
> > > > +++ b/tests/xfs/832.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 832
> > > > +Silence is golden
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 6/8] fsx: add support for XFS_IOC_ALLOCSP
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (4 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 5/8] xfs: regression test for allocsp handing out stale disk contents Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-11 21:50 ` [PATCH 7/8] iogen: upgrade to fallocate Darrick J. Wong
` (2 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Add support for this old ioctl before we remove it.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
ltp/fsx.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 108 insertions(+), 2 deletions(-)
diff --git a/ltp/fsx.c b/ltp/fsx.c
index 12c2cc33..520e53a2 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -111,6 +111,7 @@ enum {
OP_CLONE_RANGE,
OP_DEDUPE_RANGE,
OP_COPY_RANGE,
+ OP_ALLOCSP,
OP_MAX_FULL,
/* integrity operations */
@@ -165,6 +166,7 @@ int randomoplen = 1; /* -O flag disables it */
int seed = 1; /* -S flag */
int mapped_writes = 1; /* -W flag disables */
int fallocate_calls = 1; /* -F flag disables */
+int allocsp_calls = 1; /* -F flag disables */
int keep_size_calls = 1; /* -K flag disables */
int punch_hole_calls = 1; /* -H flag disables */
int zero_range_calls = 1; /* -z flag disables */
@@ -268,6 +270,7 @@ static const char *op_names[] = {
[OP_DEDUPE_RANGE] = "dedupe_range",
[OP_COPY_RANGE] = "copy_range",
[OP_FSYNC] = "fsync",
+ [OP_ALLOCSP] = "allocsp",
};
static const char *op_name(int operation)
@@ -410,6 +413,15 @@ logdump(void)
if (overlap)
prt("\t******WWWW");
break;
+ case OP_ALLOCSP:
+ down = lp->args[1] < lp->args[2];
+ prt("ALLOCSP %s\tfrom 0x%x to 0x%x",
+ down ? "DOWN" : "UP", lp->args[2], lp->args[1]);
+ overlap = badoff >= lp->args[1 + !down] &&
+ badoff < lp->args[1 + !!down];
+ if (overlap)
+ prt("\t******NNNN");
+ break;
case OP_FALLOCATE:
/* 0: offset 1: length 2: where alloced */
prt("FALLOC 0x%x thru 0x%x\t(0x%x bytes) ",
@@ -1695,6 +1707,51 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
}
#endif
+#ifdef XFS_IOC_ALLOCSP
+/* allocsp is an old Irix ioctl that either truncates or does extending preallocaiton */
+void
+do_allocsp(unsigned new_size)
+{
+ struct xfs_flock64 fl;
+
+ if (new_size > biggest) {
+ biggest = new_size;
+ if (!quiet && testcalls > simulatedopcount)
+ prt("allocsping to largest ever: 0x%x\n", new_size);
+ }
+
+ log4(OP_ALLOCSP, 0, new_size, FL_NONE);
+
+ if (new_size > file_size)
+ memset(good_buf + file_size, '\0', new_size - file_size);
+ file_size = new_size;
+
+ if (testcalls <= simulatedopcount)
+ return;
+
+ if ((progressinterval && testcalls % progressinterval == 0) ||
+ (debug && (monitorstart == -1 || monitorend == -1 ||
+ new_size <= monitorend)))
+ prt("%lld allocsp\tat 0x%x\n", testcalls, new_size);
+
+ fl.l_whence = SEEK_SET;
+ fl.l_start = new_size;
+ fl.l_len = 0;
+
+ if (ioctl(fd, XFS_IOC_ALLOCSP, &fl) == -1) {
+ prt("allocsp: 0x%x\n", new_size);
+ prterr("do_allocsp: allocsp");
+ report_failure(161);
+ }
+}
+#else
+void
+do_allocsp(unsigned new_isize)
+{
+ return;
+}
+#endif
+
#ifdef HAVE_LINUX_FALLOC_H
/* fallocate is basically a no-op unless extending, then a lot like a truncate */
void
@@ -2040,6 +2097,8 @@ test(void)
if (fallocate_calls && size && keep_size_calls)
keep_size = random() % 2;
break;
+ case OP_ALLOCSP:
+ break;
case OP_ZERO_RANGE:
if (zero_range_calls && size && keep_size_calls)
keep_size = random() % 2;
@@ -2066,6 +2125,12 @@ test(void)
if (!mapped_writes)
op = OP_WRITE;
break;
+ case OP_ALLOCSP:
+ if (!allocsp_calls) {
+ log4(OP_FALLOCATE, 0, size, FL_SKIPPED);
+ goto out;
+ }
+ break;
case OP_FALLOCATE:
if (!fallocate_calls) {
log4(OP_FALLOCATE, offset, size, FL_SKIPPED);
@@ -2141,6 +2206,10 @@ test(void)
dotruncate(size);
break;
+ case OP_ALLOCSP:
+ do_allocsp(size);
+ break;
+
case OP_FALLOCATE:
TRIM_OFF_LEN(offset, size, maxfilelen);
do_preallocate(offset, size, keep_size);
@@ -2270,8 +2339,8 @@ usage(void)
" -U: Use the IO_URING system calls, -U excludes -A\n"
#endif
" -D startingop: debug output starting at specified operation\n"
-#ifdef HAVE_LINUX_FALLOC_H
-" -F: Do not use fallocate (preallocation) calls\n"
+#if defined(HAVE_LINUX_FALLOC_H) || defined(XFS_IOC_ALLOCSP)
+" -F: Do not use fallocate (preallocation) or XFS_IOC_ALLOCSP calls\n"
#endif
#ifdef FALLOC_FL_PUNCH_HOLE
" -H: Do not use punch hole calls\n"
@@ -2587,6 +2656,41 @@ __test_fallocate(int mode, const char *mode_str)
#endif
}
+int
+test_allocsp()
+{
+#ifdef XFS_IOC_ALLOCSP
+ struct xfs_flock64 fl;
+ int ret = 0;
+
+ if (lite)
+ return 0;
+
+ fl.l_whence = SEEK_SET;
+ fl.l_start = 1;
+ fl.l_len = 0;
+
+ ret = ioctl(fd, XFS_IOC_ALLOCSP, &fl);
+ if (ret == -1 && (errno == ENOTTY || errno == EOPNOTSUPP)) {
+ if (!quiet)
+ fprintf(stderr,
+ "main: filesystem does not support "
+ "XFS_IOC_ALLOCSP, disabling!\n");
+ return 0;
+ }
+
+ ret = ftruncate(fd, file_size);
+ if (ret) {
+ warn("main: ftruncate");
+ exit(132);
+ }
+
+ return 1;
+#else
+ return 0;
+#endif
+}
+
static struct option longopts[] = {
{"replay-ops", required_argument, 0, 256},
{"record-ops", optional_argument, 0, 255},
@@ -2972,6 +3076,8 @@ main(int argc, char **argv)
if (fallocate_calls)
fallocate_calls = test_fallocate(0);
+ if (allocsp_calls)
+ allocsp_calls = test_allocsp(0);
if (keep_size_calls)
keep_size_calls = test_fallocate(FALLOC_FL_KEEP_SIZE);
if (punch_hole_calls)
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 7/8] iogen: upgrade to fallocate
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (5 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 6/8] fsx: add support for XFS_IOC_ALLOCSP Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-16 7:01 ` Eryu Guan
2022-01-11 21:50 ` [PATCH 8/8] alloc: " Darrick J. Wong
2022-01-16 7:16 ` [PATCHSET 0/8] fstests: random fixes Eryu Guan
8 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Update this utility to use fallocate to preallocate/reserve space to a
file so that we're not so dependent on legacy XFS ioctls.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
ltp/iogen.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/ltp/iogen.c b/ltp/iogen.c
index 2b6644d5..6c2c1534 100644
--- a/ltp/iogen.c
+++ b/ltp/iogen.c
@@ -928,7 +928,15 @@ bozo!
fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
/* non-zeroing reservation */
-#ifdef XFS_IOC_RESVSP
+#if defined(HAVE_FALLOCATE)
+ if (fallocate(fd, 0, 0, nbytes) == -1) {
+ fprintf(stderr,
+ "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
+ TagName, nbytes, path, SYSERR, errno);
+ close(fd);
+ return -1;
+ }
+#elif defined(XFS_IOC_RESVSP)
if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
fprintf(stderr,
"iogen%s: Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
@@ -936,8 +944,7 @@ bozo!
close(fd);
return -1;
}
-#else
-#ifdef F_RESVSP
+#elif defined(F_RESVSP)
if( fcntl( fd, F_RESVSP, &f ) == -1) {
fprintf(stderr,
"iogen%s: Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
@@ -946,8 +953,7 @@ bozo!
return -1;
}
#else
-bozo!
-#endif
+# error Dont know how to reserve space!
#endif
}
@@ -962,7 +968,15 @@ bozo!
(long long)f.l_len);*/
/* zeroing reservation */
-#ifdef XFS_IOC_ALLOCSP
+#if defined(HAVE_FALLOCATE)
+ if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
+ fprintf(stderr,
+ "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
+ TagName, nbytes, path, SYSERR, errno);
+ close(fd);
+ return -1;
+ }
+#elif defined(XFS_IOC_ALLOCSP)
if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
fprintf(stderr,
"iogen%s: Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
@@ -970,8 +984,7 @@ bozo!
close(fd);
return -1;
}
-#else
-#ifdef F_ALLOCSP
+#elif defined(F_ALLOCSP)
if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
fprintf(stderr,
"iogen%s: Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
@@ -980,8 +993,7 @@ bozo!
return -1;
}
#else
-bozo!
-#endif
+# error Dont know how to (pre)allocate space!
#endif
}
#endif
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 7/8] iogen: upgrade to fallocate
2022-01-11 21:50 ` [PATCH 7/8] iogen: upgrade to fallocate Darrick J. Wong
@ 2022-01-16 7:01 ` Eryu Guan
2022-01-17 17:15 ` Darrick J. Wong
0 siblings, 1 reply; 23+ messages in thread
From: Eryu Guan @ 2022-01-16 7:01 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests
On Tue, Jan 11, 2022 at 01:50:46PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Update this utility to use fallocate to preallocate/reserve space to a
> file so that we're not so dependent on legacy XFS ioctls.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> ltp/iogen.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
>
> diff --git a/ltp/iogen.c b/ltp/iogen.c
> index 2b6644d5..6c2c1534 100644
> --- a/ltp/iogen.c
> +++ b/ltp/iogen.c
> @@ -928,7 +928,15 @@ bozo!
> fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
>
> /* non-zeroing reservation */
> -#ifdef XFS_IOC_RESVSP
> +#if defined(HAVE_FALLOCATE)
> + if (fallocate(fd, 0, 0, nbytes) == -1) {
Seems this is not a identical replacement for XFS_IOC_RESVSP (is that
what we want here?), as fallocate(2) here zeros space and change i_size
as well.
And from xfsctl(3), after XFS_IOC_RESVSP "The blocks are allocated, but
not zeroed, and the file size does not change." And the comments above
indicates "non-zeroing reservation" as well.
If identical replacement is not required, we could drop "non-zeroing"
part, but add FALLOC_FL_KEEP_SIZE?
Thanks,
Eryu
> + fprintf(stderr,
> + "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
> + TagName, nbytes, path, SYSERR, errno);
> + close(fd);
> + return -1;
> + }
> +#elif defined(XFS_IOC_RESVSP)
> if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
> fprintf(stderr,
> "iogen%s: Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
> @@ -936,8 +944,7 @@ bozo!
> close(fd);
> return -1;
> }
> -#else
> -#ifdef F_RESVSP
> +#elif defined(F_RESVSP)
> if( fcntl( fd, F_RESVSP, &f ) == -1) {
> fprintf(stderr,
> "iogen%s: Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
> @@ -946,8 +953,7 @@ bozo!
> return -1;
> }
> #else
> -bozo!
> -#endif
> +# error Dont know how to reserve space!
> #endif
> }
>
> @@ -962,7 +968,15 @@ bozo!
> (long long)f.l_len);*/
>
> /* zeroing reservation */
> -#ifdef XFS_IOC_ALLOCSP
> +#if defined(HAVE_FALLOCATE)
> + if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
> + fprintf(stderr,
> + "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
> + TagName, nbytes, path, SYSERR, errno);
> + close(fd);
> + return -1;
> + }
> +#elif defined(XFS_IOC_ALLOCSP)
> if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
> fprintf(stderr,
> "iogen%s: Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> @@ -970,8 +984,7 @@ bozo!
> close(fd);
> return -1;
> }
> -#else
> -#ifdef F_ALLOCSP
> +#elif defined(F_ALLOCSP)
> if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
> fprintf(stderr,
> "iogen%s: Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> @@ -980,8 +993,7 @@ bozo!
> return -1;
> }
> #else
> -bozo!
> -#endif
> +# error Dont know how to (pre)allocate space!
> #endif
> }
> #endif
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 7/8] iogen: upgrade to fallocate
2022-01-16 7:01 ` Eryu Guan
@ 2022-01-17 17:15 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-17 17:15 UTC (permalink / raw)
To: Eryu Guan; +Cc: guaneryu, linux-xfs, fstests
On Sun, Jan 16, 2022 at 03:01:35PM +0800, Eryu Guan wrote:
> On Tue, Jan 11, 2022 at 01:50:46PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Update this utility to use fallocate to preallocate/reserve space to a
> > file so that we're not so dependent on legacy XFS ioctls.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > ltp/iogen.c | 32 ++++++++++++++++++++++----------
> > 1 file changed, 22 insertions(+), 10 deletions(-)
> >
> >
> > diff --git a/ltp/iogen.c b/ltp/iogen.c
> > index 2b6644d5..6c2c1534 100644
> > --- a/ltp/iogen.c
> > +++ b/ltp/iogen.c
> > @@ -928,7 +928,15 @@ bozo!
> > fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
> >
> > /* non-zeroing reservation */
> > -#ifdef XFS_IOC_RESVSP
> > +#if defined(HAVE_FALLOCATE)
> > + if (fallocate(fd, 0, 0, nbytes) == -1) {
>
> Seems this is not a identical replacement for XFS_IOC_RESVSP (is that
> what we want here?), as fallocate(2) here zeros space and change i_size
> as well.
Doh, you're right, the second parameter should follow what the kernel
does and specify FALLOC_FL_KEEP_SIZE. I'll fix that and resubmit.
Thanks for taking the rest of the series. :)
--D
> And from xfsctl(3), after XFS_IOC_RESVSP "The blocks are allocated, but
> not zeroed, and the file size does not change." And the comments above
> indicates "non-zeroing reservation" as well.
>
> If identical replacement is not required, we could drop "non-zeroing"
> part, but add FALLOC_FL_KEEP_SIZE?
>
> Thanks,
> Eryu
>
> > + fprintf(stderr,
> > + "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
> > + TagName, nbytes, path, SYSERR, errno);
> > + close(fd);
> > + return -1;
> > + }
> > +#elif defined(XFS_IOC_RESVSP)
> > if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
> > fprintf(stderr,
> > "iogen%s: Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
> > @@ -936,8 +944,7 @@ bozo!
> > close(fd);
> > return -1;
> > }
> > -#else
> > -#ifdef F_RESVSP
> > +#elif defined(F_RESVSP)
> > if( fcntl( fd, F_RESVSP, &f ) == -1) {
> > fprintf(stderr,
> > "iogen%s: Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
> > @@ -946,8 +953,7 @@ bozo!
> > return -1;
> > }
> > #else
> > -bozo!
> > -#endif
> > +# error Dont know how to reserve space!
> > #endif
> > }
> >
> > @@ -962,7 +968,15 @@ bozo!
> > (long long)f.l_len);*/
> >
> > /* zeroing reservation */
> > -#ifdef XFS_IOC_ALLOCSP
> > +#if defined(HAVE_FALLOCATE)
> > + if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
> > + fprintf(stderr,
> > + "iogen%s: Could not fallocate %d bytes in file %s: %s (%d)\n",
> > + TagName, nbytes, path, SYSERR, errno);
> > + close(fd);
> > + return -1;
> > + }
> > +#elif defined(XFS_IOC_ALLOCSP)
> > if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
> > fprintf(stderr,
> > "iogen%s: Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> > @@ -970,8 +984,7 @@ bozo!
> > close(fd);
> > return -1;
> > }
> > -#else
> > -#ifdef F_ALLOCSP
> > +#elif defined(F_ALLOCSP)
> > if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
> > fprintf(stderr,
> > "iogen%s: Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> > @@ -980,8 +993,7 @@ bozo!
> > return -1;
> > }
> > #else
> > -bozo!
> > -#endif
> > +# error Dont know how to (pre)allocate space!
> > #endif
> > }
> > #endif
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 8/8] alloc: upgrade to fallocate
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (6 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 7/8] iogen: upgrade to fallocate Darrick J. Wong
@ 2022-01-11 21:50 ` Darrick J. Wong
2022-01-16 7:16 ` [PATCHSET 0/8] fstests: random fixes Eryu Guan
8 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2022-01-11 21:50 UTC (permalink / raw)
To: djwong, guaneryu; +Cc: linux-xfs, fstests, guan
From: Darrick J. Wong <djwong@kernel.org>
Update this utility to use fallocate to preallocate/reserve space to a
file so that we're not so dependent on legacy XFS ioctls.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
src/alloc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 55 insertions(+), 11 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index 57a91f81..4a446ca8 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -118,6 +118,50 @@ bozo!
}
}
+#ifdef HAVE_FALLOCATE
+# define USE_LINUX_PREALLOCATE
+enum linux_opno {
+ FREESP = 0,
+ ALLOCSP,
+ UNRESVSP,
+ RESVSP,
+};
+
+/* emulate the irix preallocation functions with linux vfs calls */
+static int
+linux_preallocate(
+ int fd,
+ enum linux_opno opno,
+ const struct flock64 *f)
+{
+ struct stat sbuf;
+ int ret;
+
+ assert(f->l_whence == SEEK_SET);
+
+ switch (opno) {
+ case FREESP:
+ return ftruncate(fd, f->l_start);
+ case ALLOCSP:
+ ret = fstat(fd, &sbuf);
+ if (ret)
+ return ret;
+
+ return fallocate(fd, 0, sbuf.st_size,
+ f->l_start - sbuf.st_size);
+ case UNRESVSP:
+ return fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+ f->l_start, f->l_len);
+ case RESVSP:
+ return fallocate(fd, FALLOC_FL_KEEP_SIZE, f->l_start, f->l_len);
+ }
+
+ /* should never get here */
+ errno = EINVAL;
+ return -1;
+}
+#endif
+
int
main(int argc, char **argv)
{
@@ -136,23 +180,23 @@ main(int argc, char **argv)
"resvsp" };
int opno;
- /* Assume that if we have FREESP64 then we have the rest */
-#ifdef XFS_IOC_FREESP64
+#if defined(HAVE_FALLOCATE)
+ /* see static function above */
+#elif defined(XFS_IOC_FREESP64)
#define USE_XFSCTL
+ /* Assume that if we have FREESP64 then we have the rest */
static int optab[] = { XFS_IOC_FREESP64,
XFS_IOC_ALLOCSP64,
XFS_IOC_UNRESVSP64,
XFS_IOC_RESVSP64 };
-#else
-#ifdef F_FREESP64
+#elif defined(F_FREESP64)
#define USE_FCNTL
static int optab[] = { F_FREESP64,
F_ALLOCSP64,
F_UNRESVSP64,
F_RESVSP64 };
#else
-bozo!
-#endif
+# error Dont know how to preallocate space!
#endif
int rflag = 0;
struct statvfs64 svfs;
@@ -311,14 +355,14 @@ bozo!
opnames[opno], (long long)off, (long long)len);
f.l_len = len;
-#ifdef USE_XFSCTL
+#if defined(USE_LINUX_PREALLOCATE)
+ c = linux_preallocate(fd, opno, &f);
+#elif defined(USE_XFSCTL)
c = xfsctl(filename, fd, optab[opno], &f);
-#else
-#ifdef USE_FCNTL
+#elif defined(USE_FCNTL)
c = fcntl(fd, optab[opno], &f);
#else
-bozo!
-#endif
+# error Dont know how to preallocate space!
#endif
if (c < 0) {
perror(opnames[opno]);
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCHSET 0/8] fstests: random fixes
2022-01-11 21:50 [PATCHSET 0/8] fstests: random fixes Darrick J. Wong
` (7 preceding siblings ...)
2022-01-11 21:50 ` [PATCH 8/8] alloc: " Darrick J. Wong
@ 2022-01-16 7:16 ` Eryu Guan
8 siblings, 0 replies; 23+ messages in thread
From: Eryu Guan @ 2022-01-16 7:16 UTC (permalink / raw)
To: Darrick J. Wong
Cc: guaneryu, xuyang2018.jy, Theodore Ts'o, linux-xfs, fstests
On Tue, Jan 11, 2022 at 01:50:08PM -0800, Darrick J. Wong wrote:
> Hi all,
>
> Here is a pile of random fstests fixes: a couple to fix xfs_scrub
> unicode detection; one to fix xfs/220 so that it tests QUOTARM again;
> the usual adjustments to fstests to accomodate behavior changes added to
> 5.17; a regression test for CVE-2021-4155; and cleanups to make fstests
> less dependent on XFS_IOC_ALLOCSP/FREESP since those are going away for
> 5.17.
Thanks for all the fixes and new test! I've applied all but the changes
to iogen.c. (9/8 included).
Thanks,
Eryu
>
> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
>
> This is an extraordinary way to destroy everything. Enjoy!
> Comments and questions are, as always, welcome.
>
> --D
>
> kernel git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes
>
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes
>
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
> ---
> .gitignore | 1
> common/rc | 19 ++++++---
> common/xfs | 12 +++++
> ltp/fsx.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++-
> ltp/iogen.c | 32 ++++++++++----
> src/Makefile | 2 -
> src/alloc.c | 66 +++++++++++++++++++++++++-----
> src/allocstale.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/xfs/130 | 6 ++-
> tests/xfs/130.out | 1
> tests/xfs/220 | 30 ++++++++++----
> tests/xfs/308 | 5 --
> tests/xfs/308.out | 2 -
> tests/xfs/832 | 56 +++++++++++++++++++++++++
> tests/xfs/832.out | 2 +
> 15 files changed, 418 insertions(+), 43 deletions(-)
> create mode 100644 src/allocstale.c
> create mode 100755 tests/xfs/832
> create mode 100644 tests/xfs/832.out
^ permalink raw reply [flat|nested] 23+ messages in thread