Re: [PATCH] xfs: optimise xfs_mod_icount/ifree when delta < 0

From: Christoph Hellwig
Date: Thu Nov 21 2019 - 01:56:54 EST


> +static void
> xfs_sb_mod8(
> uint8_t *field,
> int8_t delta)
> {
> int8_t counter = *field;
>
> + if (!delta)
> + return;
> counter += delta;
> + ASSERT(counter >= 0);
> *field = counter;
> }

I'd almost find it easier to keep the != 0 check in the caller, and
in fact also move the assert there and just kill these helpers, e.g.

if (tp->t_imaxpct_delta != 0) {
mp->m_sb.sb_imax_pct += tp->t_imaxpct_delta;
ASSERT(mp->m_sb.sb_imax_pct >= 0);
}
if (tp->t_rextsize_delta != 0) {
mp->m_sb.sb_rextsize += tp->t_rextsize_delta;
ASSERT(mp->m_sb.sb_rextsize >= 0);
}

While this is 2 more lines per counter it is a lot more explicit and
easier to follow.

> if (idelta) {
> - error = xfs_mod_icount(mp, idelta);
> - if (error)
> - goto out_undo_fdblocks;
> + percpu_counter_add_batch(&mp->m_icount, idelta,
> + XFS_ICOUNT_BATCH);
> + if (idelta < 0)
> + ASSERT(__percpu_counter_compare(&mp->m_icount, 0,
> + XFS_ICOUNT_BATCH) >= 0)

And here I wonder if keeping single use helpers wouldn't have been a
little nicer. Not that it really matters.