On Thu, Mar 17, 2022 at 01:22:19PM +1100, Dave Chinner wrote:
> On Wed, Mar 16, 2022 at 12:41:08PM -0500, Eric Sandeen wrote:
> > On 3/14/22 1:09 PM, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > >
> > > Historically, the quota warning counter was never incremented on a
> > > softlimit violation, and hence was never enforced. Now that the counter
> > > works, the default of 5 warnings is getting enforced, which is a
> > > breakage that people aren't used to. In the interest of not introducing
> > > new fail to things that used to work, make the default warning limit of
> > > zero, and make zero mean there is no limit.
> > >
> > > Sorta-fixes: 4b8628d57b72 ("xfs: actually bump warning counts when we send warnings")
> > > Reported-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Darrick and I talked about this offline a bit yesterday, and I think
> > we reached an understanding/agreement on this ....
> >
> > While this patch will solve the problem of low warning thresholds
> > rendering timer thresholds useless, I'm still of the opinion that
> > this is not a feature to fix, but an inadvertent/broken behavior to
> > remove.
> >
> > The concept of a warning limit in xfs quota has been documented as
> > unimplemented for about 20+ years. Digging through ancient IRIX docs,
> > the intent may have been to warn once per login session
> > (which would make more sense with the current limit of 5.) However,
> > nothing can be found in code archives to indicate that the warning
> > counter was ever bumped by anything (until the semi-recent change in
> > Linux.)
> >
> > This feature is still documented as unimplemented in the xfs_quota
> > man page.
> >
> > And although there are skeletal functions to manipulate warning limits
> > in xfs_quota, they cannot be disabled, and the interface differs from
> > timer limits, so is barely usable.
> >
> > There is no concept of a "warning limit" in non-xfs quota tools, either.
> >
> > There is no documentation on what constitutes a warning event, or when
> > it should be incremented.
> >
> > tl;dr: While the warning counter bump has been upstream for some time
> > now, I think we can argue that that does not constitute a feature that
> > needs fixing or careful deprecation; TBH it looks more like a bug that
> > should be fixed by removing the increment altogether.
> >
> > And then I think we can agree that if warning limits hae been documented
> > as unimplemented for 20+ years, we can also just remove any other code
> > that is related to this unimplemented feature.
>
> Sounds fine to me. THe less untested, undefined legacy code with
> custom user APIs we have to carry around the better. Remove it all
> before someone starts poking at it with a sharp stick and finds a
> zany zero-day....
LOLYUP.
Hey Catherine, are you interested in /removing/ the quota warning limit
code from XFS? Note: just the limits, not the actually issuance of
quota warnings (xfs_quota_warn) nor the warning counter itself.
I think a good place to start would be to remove the 'warn' field from
struct xfs_quota_limits, and then remove code as necessary to fix all
the compilation errors. I think you can leave the actual warning
counter itself (struct xfs_dquot_res.warnings) since it (roughly) tracks
how many times we've sent a warning over netlink to ... wherever they
go.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
