Re: [PATCH 10/11] xfs: enable bigtime for quota timers

Amir Goldstein
2020-08-18T13:58:50Z

On Tue, Aug 18, 2020 at 1:57 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> 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@oracle.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

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.

Thanks,
Amir.