Re: [PATCH 23/26] implement generic xfs_btree_delete/delrec

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 04, 2008 at 03:35:50AM +0200, Christoph Hellwig wrote:
> Make the btree delete code generic.  Based on a patch from David Chinner
> with lots of changes to follow the original btree implementations more
> closely.  While this loses some of the generic helper routines for
> inserting/moving/removing records it also solves some of the one off
> bugs in the original code and makes it easier to verify.
.....
> +STATIC int
> +xfs_btree_update_root(
> +	struct xfs_btree_cur	*cur,
> +	struct xfs_buf		*bp,
> +	int			level,
> +	union xfs_btree_ptr	*newroot)
> +{
> +	int			error;
> +
> +	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> +	XFS_BTREE_STATS_INC(cur, killroot);
> +
> +#ifdef DEBUG
> +	if (cur->bc_btnum == XFS_BTNUM_BNO || cur->bc_btnum == XFS_BTNUM_CNT) {
> +		struct xfs_buf		*agbp = cur->bc_private.a.agbp;
> +		struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
> +
> +		ASSERT(be32_to_cpu(agf->agf_roots[cur->bc_btnum]) ==
> +		       XFS_DADDR_TO_AGBNO(cur->bc_mp, XFS_BUF_ADDR(bp)));
> +	} else if (cur->bc_btnum == XFS_BTNUM_INO) {
> +		struct xfs_buf		*agbp = cur->bc_private.a.agbp;
> +		struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
> +		xfs_fsblock_t		fsbno;
> +
> +		fsbno = XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_private.a.agno,
> +				       be32_to_cpu(agi->agi_root));
> +
> +		ASSERT(fsbno == XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)));
> +	}
> +#endif

That's kind of messy - can it be pushed out to a separate debug only
function? Also, why do we check the inobt against a long pointer,
and the allocbt against a short pointer? both are short pointer
btrees, so it doesn't really make sense to me to have different
checks on them...

Otherwise looks pretty good.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com