On Fri, 2011-01-21 at 04:22 -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>
Two comments below. First one takes a little thought,
the second is nothing. I'm OK with the change as-is.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
> Index: xfs/fs/xfs/xfs_alloc.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_alloc.c 2011-01-03 13:11:28.893004699 +0100
. . .
> +
> + 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)));
> +
You might as well make use of the return value here.
It looks like the callers of xfs_alloc_ag_vextent()
are prepared to handle EFSCORRUPTED already. I
realize that changes the behavior of the code and
doesn't just refactor it, and I haven't really
followed up exhaustively on the implications.
> + /*
> + * 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,
> + -((long)(args->len)));
How about: -(long) args->len
> }
> +
> + XFS_STATS_INC(xs_allocx);
> + XFS_STATS_ADD(xs_allocb, args->len);
> return 0;
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
