On Tue, Aug 18, 2020 at 04:58:35PM +0300, Amir Goldstein wrote:
> On Tue, Aug 18, 2020 at 1:57 AM Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote:
> >
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Enable the bigtime feature for quota timers. We decrease the accuracy
> > of the timers to ~4s in exchange for being able to set timers up to the
> > bigtime maximum.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Minor suggestion below...
>
>
> > @@ -306,5 +327,24 @@ xfs_dquot_to_disk_timestamp(
> > __be32 *dtimer,
> > time64_t timer)
> > {
> > + /* Zero always means zero, regardless of encoding. */
> > + if (!timer) {
> > + *dtimer = cpu_to_be32(0);
> > + return;
> > + }
> > +
> > + if (dqp->q_type & XFS_DQTYPE_BIGTIME) {
> > + uint64_t t = timer;
> > +
> > + /*
> > + * Round the end of the grace period up to the nearest bigtime
> > + * interval that we support, to give users the most time to fix
> > + * the problems.
> > + */
> > + t = roundup_64(t, 1U << XFS_DQ_BIGTIME_SHIFT);
> > + *dtimer = cpu_to_be32(t >> XFS_DQ_BIGTIME_SHIFT);
> > + return;
> > + }
> > +
> > *dtimer = cpu_to_be32(timer);
> > }
>
> This suggestion has to do with elegance which is subjective...
>
> /*
> * When bigtime is enabled, we trade a few bits of precision to expand the
> * expiration timeout range to match that of big inode timestamps. The grace
> * periods stored in dquot 0 are not shifted, since they record an interval,
> * not a timestamp.
> */
> #define XFS_DQ_BIGTIME_SHIFT (2)
> #define XFS_DQ_BIGTIME_SLACK ((1U << XFS_DQ_BIGTIME_SHIFT)-1)
>
> /*
> * Round the end of the grace period up to the nearest bigtime
> * interval that we support, to give users the most time to fix
> * the problems.
> */
> uint64_t t = timer + XFS_DQ_BIGTIME_SLACK;
> *dtimer = cpu_to_be32(t >> XFS_DQ_BIGTIME_SHIFT);
>
> Take it or leave it.
Hm. Normally I don't really like open-coding a rounding operation, but
it does eliminate an integer division.
(I dunno about "SLACK", but I can't think of a better word for
imprecision...)
--D
> Thanks,
> Amir.
