On 12/31/19 7:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Refactor the open-coded test for whether or not we're over quota.
Ooh, nice. This was horrible.
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_dquot.c | 61 +++++++++++++++++++++-------------------------------
> 1 file changed, 25 insertions(+), 36 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index e50c75d9d788..54e7fdcd1d4d 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -99,6 +99,17 @@ xfs_qm_adjust_dqlimits(
> xfs_dquot_set_prealloc_limits(dq);
> }
>
> +static inline bool
> +xfs_quota_exceeded(
> + const __be64 *count,
> + const __be64 *softlimit,
> + const __be64 *hardlimit) {
why pass these all as pointers?
> +
> + if (*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit))
> + return true;
> + return *hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit);
The asymmetry bothers me a little but maybe that's just me. Is
> + if ((*softlimit && be64_to_cpup(count) > be64_to_cpup(softlimit)) ||
> + (*hardlimit && be64_to_cpup(count) > be64_to_cpup(hardlimit)))
> + return true;
> + return false;
any better? *shrug*
> +}
> +
> /*
> * Check the limits and timers of a dquot and start or reset timers
> * if necessary.
> @@ -117,6 +128,8 @@ xfs_qm_adjust_dqtimers(
> struct xfs_mount *mp,
> struct xfs_disk_dquot *d)
> {
> + bool over;
> +
> ASSERT(d->d_id);
>
> #ifdef DEBUG
> @@ -131,71 +144,47 @@ xfs_qm_adjust_dqtimers(
> be64_to_cpu(d->d_rtb_hardlimit));
> #endif
>
> + over = xfs_quota_exceeded(&d->d_bcount, &d->d_blk_softlimit,
> + &d->d_blk_hardlimit);
> if (!d->d_btimer) {
> - if ((d->d_blk_softlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_softlimit))) ||
> - (d->d_blk_hardlimit && (be64_to_cpu(d->d_bcount) > be64_to_cpu(d->d_blk_hardlimit)))) {
> + if (over) {
I wonder why we check the hard limit. Isn't exceeding the soft limit
enough to start the timer? Unrelated to the refactoring tho.
> d->d_btimer = cpu_to_be32(get_seconds() +
> mp->m_quotainfo->qi_btimelimit);
> } else {
> d->d_bwarns = 0;
> }
> } else {
> - if ((!d->d_blk_softlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_softlimit))) &&
> - (!d->d_blk_hardlimit || (be64_to_cpu(d->d_bcount) <= be64_to_cpu(d->d_blk_hardlimit)))) {
> + if (!over) {
> d->d_btimer = 0;
> }
I guess that could be
> } else if (!over) {
> d->d_btimer = 0;
> }
? but again *shrug* and that's beyond refactoring, isn't it.
