| Message ID | 1650936818-20973-1-git-send-email-sandeen@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: revert "xfs: actually bump warning counts when we send warnings" | expand |
On Mon, Apr 25, 2022 at 08:33:38PM -0500, Eric Sandeen wrote: > This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7. > > XFS quota has had the concept of a "quota warning limit" since > the earliest Irix implementation, but a mechanism for incrementing > the warning counter was never implemented, as documented in the > xfs_quota(8) man page. We do know from the historical archive that > it was never incremented at runtime during quota reservation > operations. > > With this commit, the warning counter quickly increments for every > allocation attempt after the user has crossed a quote soft > limit threshold, and this in turn transitions the user to hard > quota failures, rendering soft quota thresholds and timers useless. > This was reported as a regression by users. > > Because the intended behavior of this warning counter has never been > understood or documented, and the result of this change is a regression > in soft quota functionality, revert this commit to make soft quota > limits and timers operable again. > > Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) > Signed-off-by: Eric Sandeen <sandeen@redhat.com> Assuming you're also going to send a patch to nuke xfs/144 is on its way... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/xfs_trans_dquot.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index 9ba7e6b..ebe2c22 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -603,7 +603,6 @@ > return QUOTA_NL_ISOFTLONGWARN; > } > > - res->warnings++; > return QUOTA_NL_ISOFTWARN; > } > > -- > 1.8.3.1 >
> On Apr 25, 2022, at 6:33 PM, Eric Sandeen <sandeen@redhat.com> wrote: > > This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7. > > XFS quota has had the concept of a "quota warning limit" since > the earliest Irix implementation, but a mechanism for incrementing > the warning counter was never implemented, as documented in the > xfs_quota(8) man page. We do know from the historical archive that > it was never incremented at runtime during quota reservation > operations. > > With this commit, the warning counter quickly increments for every > allocation attempt after the user has crossed a quote soft > limit threshold, and this in turn transitions the user to hard > quota failures, rendering soft quota thresholds and timers useless. > This was reported as a regression by users. > > Because the intended behavior of this warning counter has never been > understood or documented, and the result of this change is a regression > in soft quota functionality, revert this commit to make soft quota > limits and timers operable again. > > Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) > Signed-off-by: Eric Sandeen <sandeen@redhat.com> This looks fine to me. I’m also happy to work on removing the rest of the quota warning infrastructure. Reviewed-by: Catherine Hoang <catherine.hoang@oracle.com> > --- > fs/xfs/xfs_trans_dquot.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c > index 9ba7e6b..ebe2c22 100644 > --- a/fs/xfs/xfs_trans_dquot.c > +++ b/fs/xfs/xfs_trans_dquot.c > @@ -603,7 +603,6 @@ > return QUOTA_NL_ISOFTLONGWARN; > } > > - res->warnings++; > return QUOTA_NL_ISOFTWARN; > } > > -- > 1.8.3.1 >
On Tue, Apr 26, 2022 at 04:25:54AM +0000, Catherine Hoang wrote: > > On Apr 25, 2022, at 6:33 PM, Eric Sandeen <sandeen@redhat.com> wrote: > > > > This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7. > > > > XFS quota has had the concept of a "quota warning limit" since > > the earliest Irix implementation, but a mechanism for incrementing > > the warning counter was never implemented, as documented in the > > xfs_quota(8) man page. We do know from the historical archive that > > it was never incremented at runtime during quota reservation > > operations. > > > > With this commit, the warning counter quickly increments for every > > allocation attempt after the user has crossed a quote soft > > limit threshold, and this in turn transitions the user to hard > > quota failures, rendering soft quota thresholds and timers useless. > > This was reported as a regression by users. > > > > Because the intended behavior of this warning counter has never been > > understood or documented, and the result of this change is a regression > > in soft quota functionality, revert this commit to make soft quota > > limits and timers operable again. > > > > Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > This looks fine to me. I’m also happy to work on removing the rest of the > quota warning infrastructure. > > Reviewed-by: Catherine Hoang <catherine.hoang@oracle.com> Thanks, Catherine! Cheers, Dave.
On 4/25/22 9:12 PM, Darrick J. Wong wrote: > On Mon, Apr 25, 2022 at 08:33:38PM -0500, Eric Sandeen wrote: >> This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7. >> >> XFS quota has had the concept of a "quota warning limit" since >> the earliest Irix implementation, but a mechanism for incrementing >> the warning counter was never implemented, as documented in the >> xfs_quota(8) man page. We do know from the historical archive that >> it was never incremented at runtime during quota reservation >> operations. >> >> With this commit, the warning counter quickly increments for every >> allocation attempt after the user has crossed a quote soft >> limit threshold, and this in turn transitions the user to hard >> quota failures, rendering soft quota thresholds and timers useless. >> This was reported as a regression by users. >> >> Because the intended behavior of this warning counter has never been >> understood or documented, and the result of this change is a regression >> in soft quota functionality, revert this commit to make soft quota >> limits and timers operable again. >> >> Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > Assuming you're also going to send a patch to nuke xfs/144 is on its > way... Sure thing :)
Greeting,
FYI, we noticed the following commit (built with gcc-11):
commit: 720da41fc973c906b83254ac886d90b6a57a5fa4 ("[PATCH] xfs: revert "xfs: actually bump warning counts when we send warnings"")
url: https://github.com/intel-lab-lkp/linux/commits/Eric-Sandeen/xfs-revert-xfs-actually-bump-warning-counts-when-we-send-warnings/20220426-093700
base: https://git.kernel.org/cgit/fs/xfs/xfs-linux.git for-next
patch link: https://lore.kernel.org/linux-xfs/1650936818-20973-1-git-send-email-sandeen@redhat.com
in testcase: xfstests
version: xfstests-x86_64-46e1b83-1_20220414
with following parameters:
disk: 4HDD
fs: xfs
test: xfs-group-14
ucode: 0x21
test-description: xfstests is a regression test suite for xfs and other files ystems.
test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz with 8G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <oliver.sang@intel.com>
xfs/144 - output mismatch (see /lkp/benchmarks/xfstests/results//xfs/144.out.bad)
--- tests/xfs/144.out 2022-04-14 12:51:49.000000000 +0000
+++ /lkp/benchmarks/xfstests/results//xfs/144.out.bad 2022-04-29 00:06:55.414480418 +0000
@@ -13,15 +13,15 @@
*** push past the soft inode limit
[ROOT] 0 0 0 00 [--------] 3 0 0 13 [--------] 0 0 0 00 [--------]
-[NAME] 0 0 0 00 [--------] 4 3 500000 01 [7 days] 0 0 0 00 [--------]
+[NAME] 0 0 0 00 [--------] 4 3 500000 00 [7 days] 0 0 0 00 [--------]
*** push further past the soft inode limit
...
(Run 'diff -u /lkp/benchmarks/xfstests/tests/xfs/144.out /lkp/benchmarks/xfstests/results//xfs/144.out.bad' to see the entire diff)
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index 9ba7e6b..ebe2c22 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -603,7 +603,6 @@ return QUOTA_NL_ISOFTLONGWARN; } - res->warnings++; return QUOTA_NL_ISOFTWARN; }
This reverts commit 4b8628d57b725b32616965e66975fcdebe008fe7. XFS quota has had the concept of a "quota warning limit" since the earliest Irix implementation, but a mechanism for incrementing the warning counter was never implemented, as documented in the xfs_quota(8) man page. We do know from the historical archive that it was never incremented at runtime during quota reservation operations. With this commit, the warning counter quickly increments for every allocation attempt after the user has crossed a quote soft limit threshold, and this in turn transitions the user to hard quota failures, rendering soft quota thresholds and timers useless. This was reported as a regression by users. Because the intended behavior of this warning counter has never been understood or documented, and the result of this change is a regression in soft quota functionality, revert this commit to make soft quota limits and timers operable again. Fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings) Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- fs/xfs/xfs_trans_dquot.c | 1 - 1 file changed, 1 deletion(-)