On Wed, Jul 01, 2020 at 09:56:21AM +0100, Christoph Hellwig wrote:
> On Tue, Jun 30, 2020 at 08:43:20AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Refactor the open-coded test for whether or not we're over quota.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_dquot.c | 95 ++++++++++++++++------------------------------------
> > 1 file changed, 30 insertions(+), 65 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index 35a113d1b42b..ef34c82c28a0 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -97,6 +97,33 @@ xfs_qm_adjust_dqlimits(
> > xfs_dquot_set_prealloc_limits(dq);
> > }
> >
> > +/*
> > + * Determine if this quota counter is over either limit and set the quota
> > + * timers as appropriate.
> > + */
> > +static inline void
> > +xfs_qm_adjust_res_timer(
> > + struct xfs_dquot_res *res,
> > + struct xfs_def_qres *dres)
> > +{
> > + bool over;
> > +
> > +#ifdef DEBUG
> > + if (res->hardlimit)
> > + ASSERT(res->softlimit <= res->hardlimit);
> > +#endif
>
> Maybe:
> ASSERRT(!res->hardlimit || res->softlimit <= res->hardlimit);
Changed.
>
> > +
> > + over = (res->softlimit && res->count > res->softlimit) ||
> > + (res->hardlimit && res->count > res->hardlimit);
> > +
> > + if (over && res->timer == 0)
> > + res->timer = ktime_get_real_seconds() + dres->timelimit;
> > + else if (!over && res->timer != 0)
> > + res->timer = 0;
> > + else if (!over && res->timer == 0)
> > + res->warnings = 0;
>
> What about:
>
> if ((res->softlimit && res->count > res->softlimit) ||
> (res->hardlimit && res->count > res->hardlimit)) {
> if (res->timer == 0)
> res->timer = ktime_get_real_seconds() + dres->timelimit;
> } else {
> if (res->timer)
> res->timer = 0;
> else
> res->warnings = 0;
> }
I don't care either way, but the last time I sent this patch out, Eric
and Amir seemed to want a flatter if structure:
https://lore.kernel.org/linux-xfs/b979d33d-361b-88cd-699c-7e5f1c621698@xxxxxxxxxxx/
https://lore.kernel.org/linux-xfs/CAOQ4uxiveTQu8_7UOvN07=P4o9hBBZTCyu4sSw5UpbrNPQL2pQ@xxxxxxxxxxxxxx/
Granted that was before I pulled the whole thing into a separate helper
function, so maybe the context is different here...?
--D
