Re: [PATCH 02/11] xfs: refactor quota expiration timer modification

Darrick J. Wong
2020-08-18T15:40:18Z

On Tue, Aug 18, 2020 at 09:48:14AM +0300, Amir Goldstein wrote:
> On Tue, Aug 18, 2020 at 1:59 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Define explicit limits on the range of quota grace period expiration
> > timeouts and refactor the code that modifies the timeouts into helpers
> > that clamp the values appropriately.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> Question below...
>
> ...
>
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index 1c542b4a5220..b16d533a6feb 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -483,9 +483,14 @@ xfs_setqlim_timer(
> > struct xfs_quota_limits *qlim,
> > s64 timer)
> > {
> > - res->timer = timer;
> > - if (qlim)
> > + if (qlim) {
> > + /* Set the length of the default grace period. */
> > + res->timer = timer;
> > qlim->time = timer;
> > + } else {
> > + /* Set the grace period expiration on a quota. */
> > + xfs_dquot_set_timeout(&res->timer, timer);
> > + }
> > }
>
> I understand why you did this. This is mid series craft, but it looks very odd
> to your average reviewer.
>
> Maybe leave a TODO comment above res->timer = timer which will be
> removed later in the series?
>
> Not critical.

<shrug> The grace period case gets changed in the very next patch, but I
suppose it wouldn't hurt to mention that in the commit log.

--D

>
> Thanks,
> Amir.