Re: [PATCH 11/12] xfs: remove struct xfs_dabuf and infrastructure

Christoph Hellwig
2011-12-17T23:30:49Z

> ASSERT(bp != NULL);
> - leaf = bp->data;
> + leaf = bp->b_addr;
> if (unlikely(leaf->hdr.info.magic != cpu_to_be16(XFS_ATTR_LEAF_MAGIC))) {
> XFS_CORRUPTION_ERROR("xfs_attr_leaf_list", XFS_ERRLEVEL_LOW,
> context->dp->i_mount, leaf);
> - xfs_da_brelse(NULL, bp);
> + xfs_trans_brelse(NULL, bp);

Wouldn't a direct call to xfs_buf_relse be the more logical replacement
for a call to xfs_da_brelse with a constant NULL transaction argument?

> int
> -xfs_attr_shortform_allfit(xfs_dabuf_t *bp, xfs_inode_t *dp)
> +xfs_attr_shortform_allfit(
> + struct xfs_buf *bp,
> + struct xfs_inode *dp)

If you'd use one more tab all arguments would be full aligned. This
also happens in a few more places.

> -xfs_attr_leaf_to_shortform(xfs_dabuf_t *bp, xfs_da_args_t *args, int forkoff)
> +xfs_attr_leaf_to_shortform(
> + struct xfs_buf *bp,
> + xfs_da_args_t *args,

Also if you touch the argument list anyway please replace xfs_da_args_t
with struct xfs_da_args.

> void
> xfs_da_state_free(xfs_da_state_t *state)
> {
> - int i;
> -
> xfs_da_state_kill_altpath(state);
> - for (i = 0; i < state->path.active; i++) {
> - if (state->path.blk[i].bp)
> - xfs_da_buf_done(state->path.blk[i].bp);
> - }
> - if (state->extravalid && state->extrablk.bp)
> - xfs_da_buf_done(state->extrablk.bp);
> #ifdef DEBUG
> memset((char *)state, 0, sizeof(*state));
> #endif /* DEBUG */
> kmem_zone_free(xfs_da_state_zone, state);

We really should kill that memset, which only defeats slab poisoning.
Not in this patch, though.