On Wed, Jul 01, 2020 at 10:51:34AM -0700, Darrick J. Wong wrote:
> 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:
I much prefer Christoph's version - I was going to suggest the same
sort of thing myself as the "flatter" version just looks needlessly
convoluted to me.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
