On Fri, Jan 21, 2011 at 04:22:29AM -0500, Christoph Hellwig wrote:
> Updating the AGF/trans counters is duplicated between allocating and
> freeing extents. Factor the code into a common helper.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good, but one minor thing:
> ASSERT(0);
> /* NOTREACHED */
> }
> - if (error)
> +
> + if (error || args->agbno == NULLAGBLOCK)
> return error;
> - /*
> - * If the allocation worked, need to change the agf structure
> - * (and log it), and the superblock.
> - */
> - if (args->agbno != NULLAGBLOCK) {
> - xfs_agf_t *agf; /* allocation group freelist header */
> - long slen = (long)args->len;
> -
> - ASSERT(args->len >= args->minlen && args->len <= args->maxlen);
> - ASSERT(!(args->wasfromfl) || !args->isfl);
> - ASSERT(args->agbno % args->alignment == 0);
> - if (!(args->wasfromfl)) {
> -
> - agf = XFS_BUF_TO_AGF(args->agbp);
> - be32_add_cpu(&agf->agf_freeblks, -(args->len));
> - xfs_trans_agblocks_delta(args->tp,
> - -((long)(args->len)));
> - args->pag->pagf_freeblks -= args->len;
> - ASSERT(be32_to_cpu(agf->agf_freeblks) <=
> - be32_to_cpu(agf->agf_length));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The assert is lost in this path.
> - xfs_alloc_log_agf(args->tp, args->agbp,
> - XFS_AGF_FREEBLKS);
> - /*
> - * Search the busylist for these blocks and mark the
> - * transaction as synchronous if blocks are found. This
> - * avoids the need to block due to a synchronous log
> - * force to ensure correct ordering as the synchronous
> - * transaction will guarantee that for us.
> - */
> - if (xfs_alloc_busy_search(args->mp, args->agno,
> - args->agbno, args->len))
> - xfs_trans_set_sync(args->tp);
> - }
> - if (!args->isfl)
> - xfs_trans_mod_sb(args->tp,
> - args->wasdel ? XFS_TRANS_SB_RES_FDBLOCKS :
> - XFS_TRANS_SB_FDBLOCKS, -slen);
> - XFS_STATS_INC(xs_allocx);
> - XFS_STATS_ADD(xs_allocb, args->len);
> +
> + ASSERT(args->len >= args->minlen);
> + ASSERT(args->len <= args->maxlen);
> + ASSERT(!args->wasfromfl || !args->isfl);
> + ASSERT(args->agbno % args->alignment == 0);
> +
> + if (!args->wasfromfl) {
> + xfs_alloc_update_counters(args->tp, args->pag, args->agbp,
> + -((long)(args->len)));
Perhaps catching the error here and adding an ASSERT(error == 0)
would be a good idea.
Otherwise,
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
