xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 11/12] xfs: remove struct xfs_dabuf and infrastructure
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Sat, 17 Dec 2011 18:30:47 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1323238703-13198-12-git-send-email-david@xxxxxxxxxxxxx>
References: <1323238703-13198-1-git-send-email-david@xxxxxxxxxxxxx> <1323238703-13198-12-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
>       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.

<Prev in Thread] Current Thread [Next in Thread>