xfs
[Top] [All Lists]

Re: [PATCH 06/11] xfs: fully initialise temp leaf in xfs_attr3_leaf_unba

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 06/11] xfs: fully initialise temp leaf in xfs_attr3_leaf_unbalance
From: Ben Myers <bpm@xxxxxxx>
Date: Wed, 22 May 2013 15:50:59 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1369123330-9579-7-git-send-email-david@xxxxxxxxxxxxx>
References: <1369123330-9579-1-git-send-email-david@xxxxxxxxxxxxx> <1369123330-9579-7-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, May 21, 2013 at 06:02:05PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> xfs_attr3_leaf_unbalance() uses a temporary buffer for recombining
> the entries in two leaves when the destination leaf requires
> compaction. The temporary buffer ends up being copied back over the
> original destination buffer, so the header in the temporary buffer
> needs to contain all the information that is in the destination
> buffer.
> 
> To make sure the temporary buffer is fully initialised, once we've
> set up the temporary incore header appropriately, write is back to
> the temporary buffer before starting to move entries around.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> ---
>  fs/xfs/xfs_attr_leaf.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
> index 0bce1b3..79ece72 100644
> --- a/fs/xfs/xfs_attr_leaf.c
> +++ b/fs/xfs/xfs_attr_leaf.c
> @@ -2181,14 +2181,24 @@ xfs_attr3_leaf_unbalance(
>               struct xfs_attr_leafblock *tmp_leaf;
>               struct xfs_attr3_icleaf_hdr tmphdr;
>  
> -             tmp_leaf = kmem_alloc(state->blocksize, KM_SLEEP);
> -             memset(tmp_leaf, 0, state->blocksize);
> -             memset(&tmphdr, 0, sizeof(tmphdr));
> +             tmp_leaf = kmem_zalloc(state->blocksize, KM_SLEEP);
> +
> +             /*
> +              * Copy the header into the temp leaf so that all the stuff
> +              * not in the incore header is present and gets copied back in
> +              * once we've moved all the entries.
> +              */
> +             memcpy(tmp_leaf, save_leaf, xfs_attr3_leaf_hdr_size(save_leaf));
>  
> +             memset(&tmphdr, 0, sizeof(tmphdr));
>               tmphdr.magic = savehdr.magic;
>               tmphdr.forw = savehdr.forw;
>               tmphdr.back = savehdr.back;
>               tmphdr.firstused = state->blocksize;
> +
> +             /* write the header to the temp buffer to initialise it */
> +             xfs_attr3_leaf_hdr_to_disk(tmp_leaf, &tmphdr);
> +

Hrm.  It seemed like the memset 0 for the incore temp header should have been
enough for the purposes of moveents.  Why did you have to add this additional
hdr_to_disk?

-Ben

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