Re: [PATCH v1 0/2] xfs: remove quota warning limits

[Dave Chinner <david@fromorbit.com>]

On Mon, Apr 25, 2022 at 01:19:35PM -0500, Eric Sandeen wrote:
> On 4/21/22 11:58 AM, Catherine Hoang wrote:
> > Hi all,
> > 
> > Based on recent discussion, it seems like there is a consensus that quota
> > warning limits should be removed from xfs quota.
> > https://lore.kernel.org/linux-xfs/94893219-b969-c7d4-4b4e-0952ef54d575@sandeen.net/
> > 
> > Warning limits in xfs quota is an unused feature that is currently
> > documented as unimplemented. These patches remove the quota warning limits
> > and cleans up any related code. 
> > 
> > Comments and feedback are appreciated!
> > 
> > Catherine
> > 
> > Catherine Hoang (2):
> >   xfs: remove quota warning limit from struct xfs_quota_limits
> >   xfs: don't set warns on the id==0 dquot
> > 
> >  fs/xfs/xfs_qm.c          |  9 ---------
> >  fs/xfs/xfs_qm.h          |  5 -----
> >  fs/xfs/xfs_qm_syscalls.c | 19 +++++--------------
> >  fs/xfs/xfs_quotaops.c    |  3 ---
> >  fs/xfs/xfs_trans_dquot.c |  3 +--
> >  5 files changed, 6 insertions(+), 33 deletions(-)
> 
> I have a question about the remaining warning counter infrastructure after these
> patches are applied.
> 
> We still have xfs_dqresv_check() incrementing the warning counter, as was added in
> 4b8628d5 "xfs: actually bump warning counts when we send warnings"
> 
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -589,6 +589,7 @@
>                         return QUOTA_NL_ISOFTLONGWARN;
>                 }
>  
> +               res->warnings++;
>                 return QUOTA_NL_ISOFTWARN;
>         }

/me reads another overnight #xfs explosion over this one line of
code and sighs.

Well, so much for hoping that there would be an amicable resolution
to this sorry saga without having to get directly involved.  I'm fed
up with watching the tantrums, the petty arguments, the refusal to
compromise, acknowledge mistakes, etc.

Enough, OK?

Commit 4b8628d5 is fundamentally broken and causes production
systems regressions - it just doesn't work in any useful way as it
stands.  Eric, send me a patch that reverts this commit, and I will
review and commit it.

Further:

- this is legacy functionality that was never implemented in Linux,
- it cannot be implemented in Linux the (useful) way it was
  implemented in Irix,
- it is documented as unimplemented,
- no use case for the functionality in Linux has been presented
  ("do something useful" is not a use case),
- no documentation has been written for it,
- no fstests coverage of the functionality exists,
- linux userspace already has quota warning infrastructure via
  netlink so just accounting warnings in the kernel is of very
  limited use,
- it broke existing production systems.

Given all this, and considering our new policy of not tolerating
unused or questionable legacy code in the XFS code base any more
(precendence: ALLOCSP), it is clear that all aspects of this quota
warning code should simply be removed ASAP.

Eric and/or Catherine, please send patches to first revert 4b8628d5
and then remove *all* of this quota warning functionality completely
(including making the user APIs see zeros on all reads and sliently
ignore all writes) before I get sufficiently annoyed to simply
remove the code directly myself.

So disappointment.

-Dave.
-- 
Dave Chinner
david@fromorbit.com