> On Apr 21, 2022, at 6:40 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > On Thu, Apr 21, 2022 at 09:58:15AM -0700, Catherine Hoang wrote: >> Quotas are not enforced on the id==0 dquot, so the quota code uses it >> to store warning limits and timeouts. Having just dropped support for >> warning limits, this field no longer has any meaning. Return -EINVAL >> for this dquot id if the fieldmask has any of the QC_*_WARNS set. >> >> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx> >> --- >> fs/xfs/xfs_qm_syscalls.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c >> index e7f3ac60ebd9..bdbd5c83b08e 100644 >> --- a/fs/xfs/xfs_qm_syscalls.c >> +++ b/fs/xfs/xfs_qm_syscalls.c >> @@ -290,6 +290,8 @@ xfs_qm_scall_setqlim( >> return -EINVAL; >> if ((newlim->d_fieldmask & XFS_QC_MASK) == 0) >> return 0; >> + if ((newlim->d_fieldmask & QC_WARNS_MASK) && id == 0) >> + return -EINVAL; > > Why would we do this for only id == 0? This will still allow > non-zero warning values to be written to dquots that have id != 0, > but I'm not sure why we'd allow this given that the previous > patch just removed all the warning limit checking? > > Which then makes me ask: why are we still reading the warning counts > from on disk dquots and writing in-memory values back to dquots? > Shouldn't xfs_dquot_to_disk() just write zeros to the warning fields > now, and xfs_dquot_from_disk() elide reading the warning counts > altogether? i.e. can we remove d_bwarns, d_iwarns and d_rtbwarns > from the struct fs_disk_quota altogether now? > > Which then raises the question of whether copy_from_xfs_dqblk() and > friends should still support warn counts up in fs/quota/quota.c...? The intent of this patchset is to only remove warning limits, not warning counts. The id == 0 dquot stores warning limits, so we no longer want warning values to be written here. Dquots with id != 0 store warning counts, so we still allow these values to be updated. In regards to whether or not warning counts should be removed, it seems like they currently do serve a purpose. Although there's some debate about this in a previous thread: https://lore.kernel.org/linux-xfs/20220303003808.GM117732@magnolia/ Catherine > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
