xfs
[Top] [All Lists]

Re: [PATCH 23/25] xfs: connect up write verifiers to new buffers

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 23/25] xfs: connect up write verifiers to new buffers
From: Phil White <pwhite@xxxxxxx>
Date: Tue, 30 Oct 2012 06:39:38 -0700
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1351146854-19343-24-git-send-email-david@xxxxxxxxxxxxx>
References: <1351146854-19343-1-git-send-email-david@xxxxxxxxxxxxx> <1351146854-19343-24-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Oct 25, 2012 at 05:34:12PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Metadata buffers that are read from disk have write verifiers
> already attached to them, but newly allocated buffers do not. Add
> appropriate write verifiers to all new metadata buffers.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_alloc.c        |    6 +--
>  fs/xfs/xfs_alloc.h        |    2 +
>  fs/xfs/xfs_alloc_btree.c  |    1 +
>  fs/xfs/xfs_attr_leaf.c    |    4 +-
>  fs/xfs/xfs_bmap.c         |    2 +
>  fs/xfs/xfs_bmap_btree.c   |    3 +-
>  fs/xfs/xfs_bmap_btree.h   |    1 +
>  fs/xfs/xfs_btree.c        |    1 +
>  fs/xfs/xfs_btree.h        |    2 +
>  fs/xfs/xfs_da_btree.c     |    3 ++
>  fs/xfs/xfs_dir2_block.c   |    2 +
>  fs/xfs/xfs_dir2_data.c    |   11 +++--
>  fs/xfs/xfs_dir2_leaf.c    |   19 ++++++---
>  fs/xfs/xfs_dir2_node.c    |   24 +++++++----
>  fs/xfs/xfs_dir2_priv.h    |    2 +
>  fs/xfs/xfs_dquot.c        |  104 
> ++++++++++++++++++++++-----------------------
>  fs/xfs/xfs_fsops.c        |    7 ++-
>  fs/xfs/xfs_ialloc.c       |    5 ++-
>  fs/xfs/xfs_ialloc.h       |    4 +-
>  fs/xfs/xfs_ialloc_btree.c |    1 +
>  fs/xfs/xfs_inode.c        |   14 +++++-
>  fs/xfs/xfs_inode.h        |    1 +
>  fs/xfs/xfs_mount.c        |    2 +-
>  fs/xfs/xfs_mount.h        |    1 +
>  24 files changed, 135 insertions(+), 87 deletions(-)
> 

A few comments:

> diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
> index bb96c55..5d56886 100644
> --- a/fs/xfs/xfs_attr_leaf.c
> +++ b/fs/xfs/xfs_attr_leaf.c
> @@ -923,7 +923,7 @@ xfs_attr_leaf_to_node(xfs_da_args_t *args)
>                                           XFS_ATTR_FORK);
>       if (error)
>               goto out;
> -     ASSERT(bp2 != NULL);
> +     bp2->b_pre_io = bp1->b_pre_io;
>       memcpy(bp2->b_addr, bp1->b_addr, XFS_LBSIZE(dp->i_mount));
>       bp1 = NULL;
>       xfs_trans_log_buf(args->trans, bp2, 0, XFS_LBSIZE(dp->i_mount) - 1);
> @@ -977,7 +977,7 @@ xfs_attr_leaf_create(
>                                           XFS_ATTR_FORK);
>       if (error)
>               return(error);
> -     ASSERT(bp != NULL);
> +     bp->b_pre_io = xfs_attr_leaf_write_verify;
>       leaf = bp->b_addr;
>       memset((char *)leaf, 0, XFS_LBSIZE(dp->i_mount));
>       hdr = &leaf->hdr;

I'm unclear as to why you're removing the asserts here.  There must be
a reason that you think bp is guaranteed to be safe, but I haven't
grasped it here.

-Phil

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