On 3/16/22 9:53 PM, Darrick J. Wong wrote:
> 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.
I think we also discussed a separate patch to simply remove the counter bump,
which is easily backportable to distros and stable kernels?
thanks,
-Eric
