On Thu, May 05, 2022 at 11:43:44AM -0700, Darrick J. Wong wrote:
> On Fri, May 06, 2022 at 02:25:55AM +0800, Zorro Lang wrote:
> > After soft limits are exceeded, within the grace time, fs quota should
> > allow more space allocation before exceeding hard limits, even if
> > allocating many small files.
> >
> > This case can cover bc37e4fb5cac (xfs: revert "xfs: actually bump
> > warning counts when we send warnings"). And will help to expose later
> > behavior changes on this side.
>
> Isn't this already covered by generic/230?
No. They're similar, but the main difference at here:
+ # Exceed the soft quota limit a bit at first
+ su $qa_user -c "$XFS_IO_PROG -f -t -c 'pwrite 0 2m' -c fsync ${file}.0" >>$seqres.full 2>&1
+ # Write more data more times under soft quota limit exhausted condition,
+ # but not reach hard limit. To make sure the it won't trigger EDQUOT.
+ for ((i=1; i<=100; i++));do
+ su "$qa_user" -c "$XFS_IO_PROG -f -c 'pwrite 0 1m' -c fsync ${file}.$i" >>$seqres.full 2>&1
+ if [ $? -ne 0 ];then
+ log "Unexpected error (type=$type)!"
+ break
+ fi
+ done
This case trys to exceed soft quota with *many writes*. We've talked about it in
IRC last month, the detailed chat log at below[1].
Thanks,
Zorro
[1]
2022-04-14 16:49 < zlang> https://lore.kernel.org/fstests/161526483668.1214319.17667836667890283825.stgit@magnolia/
2022-04-14 16:50 < zlang> djwong: About xfs/144 ^^, are we going to remove it, or change it to cover old quota warning behavior (without warn counter)?
2022-04-14 17:04 < zlang> Just due to notice your discussion in https://lore.kernel.org/linux-xfs/20220314180914.GN8224@magnolia/, and our downstream testing fails on xfs/144. So ask your plan :)
2022-04-14 22:53 < djwong> zlang: catherine is working on a patch to kill the warning counter limit feature entirely, since it's been largely busted for 20 years
2022-04-14 22:53 < djwong> so with that xfs/144 can be removed entirely
2022-04-14 23:11 < zlang> djwong: I'm wondering one thing, after the "the warning counter" feature being merged, why there's not any xfstests quota related cases fail on it
2022-04-14 23:12 < zlang> That might mean we miss some testing coverage
2022-04-14 23:13 < zlang> Maybe some code branches we never through, likes the place where you add "res->warnings++" :)
2022-04-14 23:15 < zlang> Even for us, we get the complaint from customers, before we find this behavior change by our testing
2022-04-14 23:19 < sandeen> djwong, if catherine would like to send the one-liner ASAP to make the next kernel, that would be fantastic, with a followup to remove the rest of the code ;)
2022-04-14 23:19 < zlang> So I'd like to change xfs/144 to cover old quota warning expected behavior, What do you think?
2022-04-14 23:19 < sandeen> zlang, what old quota warning expected behavior?
2022-04-14 23:21 < sandeen> to be clear, there are two notions of "warnings" here. thre is a netlink interface which can send the user over-quota notices. That has been there a long time, and exists for vfs quota too. But that is not really related to the quota warning counter/limit code, which is unique to xfs, and broken.
2022-04-14 23:22 < zlang> sandeen: https://paste.centos.org/view/c66654c0
2022-04-14 23:22 < zlang> old behavior means this testing won't fail ^^
2022-04-14 23:23 < zlang> Before, we always write one single file until exceed soft limit
2022-04-14 23:23 < zlang> We never tested write several small files until exceed soft limit
2022-04-14 23:23 < sandeen> if I am reading the test correctly, you have a soft limit of 2 megs and a hard limit of 500 megs. And you have a grace period of 6 days.
2022-04-14 23:24 < sandeen> Then you write 202 megs immediately.
2022-04-14 23:24 < zlang> Yes, create ~100 2M file
2022-04-14 23:24 < sandeen> all of this happens before the 6 days grace period expires, so all writes should succeed.
2022-04-14 23:25 < zlang> Yes
2022-04-14 23:25 < zlang> Before, we generally write one single 200M file to test the soft limit
2022-04-14 23:25 < sandeen> with the quota warning limit patch, they do not succeed. but that, IMHO, was a bug which needs to be fixed.
2022-04-14 23:25 < sandeen> oh I see.
2022-04-14 23:26 < sandeen> so you're suggesting that another test which exceeds soft quota with many writes is a good idea, and would have caught the quota warning /limit/ problem - right?
2022-04-14 23:26 < sandeen> that sounds reasonable to me
2022-04-14 23:33 < zlang> Due to the warning counter feature broke our customers' environment at first, but our regression test pass as usual.
2022-04-14 23:36 < zlang> So I hope a test can detect this behavior change, even if we really like to change that in one day, we can remove/change this test then.
2022-04-14 23:43 < zlang> And don't worry, I'll write that test after you revert that feature, just ask your plans :)
>
> --D
>
> > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > ---
> > tests/generic/690 | 125 ++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/690.out | 2 +
> > 2 files changed, 127 insertions(+)
> > create mode 100755 tests/generic/690
> > create mode 100644 tests/generic/690.out
> >
> > diff --git a/tests/generic/690 b/tests/generic/690
> > new file mode 100755
> > index 00000000..b1d055dc
> > --- /dev/null
> > +++ b/tests/generic/690
> > @@ -0,0 +1,125 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 Red Hat Inc. All Rights Reserved.
> > +#
> > +# FS QA Test 690
> > +#
> > +# Make sure filesystem quota works well, after soft limits are exceeded. The
> > +# fs quota should allow more space allocation before exceeding hard limits
> > +# and with in grace time.
> > +#
> > +# But different with other similar testing, this case trys to write many small
> > +# files, to cover bc37e4fb5cac (xfs: revert "xfs: actually bump warning counts
> > +# when we send warnings"). If there's a behavior change in one day, this case
> > +# might help to detect that too.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quota
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > + restore_project
> > + cd /
> > + rm -r -f $tmp.*
> > +}
> > +
> > +# Import common functions.
> > +. ./common/quota
> > +
> > +# real QA test starts here
> > +_supported_fs generic
> > +_require_scratch
> > +_require_quota
> > +_require_user
> > +_require_group
> > +
> > +create_project()
> > +{
> > + rm -rf $SCRATCH_MNT/t
> > + mkdir $SCRATCH_MNT/t
> > + $XFS_IO_PROG -r -c "chproj 100" -c "chattr +P" $SCRATCH_MNT/t
> > + chmod ugo+rwx $SCRATCH_MNT/t/
> > +
> > + rm -f $tmp.projects $tmp.projid
> > + if [ -f /etc/projects ];then
> > + cat /etc/projects > $tmp.projects
> > + fi
> > + if [ -f /etc/projid ];then
> > + cat /etc/projid > $tmp.projid
> > + fi
> > +
> > + cat >/etc/projects <<EOF
> > +100:$SCRATCH_MNT/t
> > +EOF
> > + cat >/etc/projid <<EOF
> > +$qa_user:100
> > +EOF
> > + PROJECT_CHANGED=1
> > +}
> > +
> > +restore_project()
> > +{
> > + if [ "$PROJECT_CHANGED" = "1" ];then
> > + rm -f /etc/projects /etc/projid
> > + if [ -f $tmp.projects ];then
> > + cat $tmp.projects > /etc/projects
> > + fi
> > + if [ -f $tmp.projid ];then
> > + cat $tmp.projid > /etc/projid
> > + fi
> > + fi
> > +}
> > +
> > +# Make sure the kernel supports project quota
> > +_scratch_mkfs >$seqres.full 2>&1
> > +_scratch_enable_pquota
> > +_qmount_option "prjquota"
> > +_qmount
> > +_require_prjquota $SCRATCH_DEV
> > +
> > +exercise()
> > +{
> > + local type=$1
> > + local file=$SCRATCH_MNT/testfile
> > +
> > + echo "= Test type=$type quota =" >>$seqres.full
> > + _scratch_unmount
> > + _scratch_mkfs >>$seqres.full 2>&1
> > + if [ "$1" = "P" ];then
> > + _scratch_enable_pquota
> > + fi
> > + _qmount
> > + if [ "$1" = "P" ];then
> > + create_project
> > + file=$SCRATCH_MNT/t/testfile
> > + fi
> > +
> > + setquota -${type} $qa_user 1M 500M 0 0 $SCRATCH_MNT
> > + setquota -${type} -t 86400 86400 $SCRATCH_MNT
> > + repquota -v -${type} $SCRATCH_MNT | grep -v "^root" >>$seqres.full 2>&1
> > + # Exceed the soft quota limit a bit at first
> > + su $qa_user -c "$XFS_IO_PROG -f -t -c 'pwrite 0 2m' -c fsync ${file}.0" >>$seqres.full 2>&1
> > + # Write more data more times under soft quota limit exhausted condition,
> > + # but not reach hard limit. To make sure the it won't trigger EDQUOT.
> > + for ((i=1; i<=100; i++));do
> > + su "$qa_user" -c "$XFS_IO_PROG -f -c 'pwrite 0 1m' -c fsync ${file}.$i" >>$seqres.full 2>&1
> > + if [ $? -ne 0 ];then
> > + log "Unexpected error (type=$type)!"
> > + break
> > + fi
> > + done
> > + repquota -v -${type} $SCRATCH_MNT | grep -v "^root" >>$seqres.full 2>&1
> > +}
> > +
> > +_qmount_option "usrquota"
> > +exercise u
> > +_qmount_option "grpquota"
> > +exercise g
> > +_qmount_option "prjquota"
> > +exercise P
> > +
> > +echo "Silence is golden"
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/690.out b/tests/generic/690.out
> > new file mode 100644
> > index 00000000..6f3723e3
> > --- /dev/null
> > +++ b/tests/generic/690.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 690
> > +Silence is golden
> > --
> > 2.31.1
> >
>
