On Wed, May 22, 2013 at 04:59:34PM -0500, Ben Myers wrote:
> On Tue, May 21, 2013 at 06:02:06PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > xfs_attr3_leaf_compact() uses a temporary buffer for compacting the
> > the entries in a leaf. It copies the the original buffer into the
> > temporary buffer, then zeros the original buffer completely. It then
> > copies the entries back into the original buffer. However, the
> > original buffer has not been correctly initialised, and so the
> > movement of the entries goes horribly wrong.
> >
> > Make sure the zeroed destination buffer is fully initialised, and
> > once we've set up the destination incore header appropriately, write
> > is back to the buffer before starting to move entries around.
> it
>
> >
> > While debugging this, the _d/_s prefixes weren't sufficient to
> > remind me what buffer was what, so rename then all _src/_dst.
>
> Yeah, it helps.
>
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_attr_leaf.c | 42 ++++++++++++++++++++++++++----------------
> > 1 file changed, 26 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c
> > index 79ece72..5b03d15 100644
> > --- a/fs/xfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/xfs_attr_leaf.c
> > @@ -1445,11 +1445,12 @@ xfs_attr3_leaf_add_work(
> > STATIC void
> > xfs_attr3_leaf_compact(
> > struct xfs_da_args *args,
> > - struct xfs_attr3_icleaf_hdr *ichdr_d,
> > + struct xfs_attr3_icleaf_hdr *ichdr_dst,
> > struct xfs_buf *bp)
> > {
> > - xfs_attr_leafblock_t *leaf_s, *leaf_d;
> > - struct xfs_attr3_icleaf_hdr ichdr_s;
> > + struct xfs_attr_leafblock *leaf_src;
> > + struct xfs_attr_leafblock *leaf_dst;
> > + struct xfs_attr3_icleaf_hdr ichdr_src;
> > struct xfs_trans *trans = args->trans;
> > struct xfs_mount *mp = trans->t_mountp;
> > char *tmpbuffer;
> > @@ -1457,29 +1458,38 @@ xfs_attr3_leaf_compact(
> > trace_xfs_attr_leaf_compact(args);
> >
> > tmpbuffer = kmem_alloc(XFS_LBSIZE(mp), KM_SLEEP);
> > - ASSERT(tmpbuffer != NULL);
> > memcpy(tmpbuffer, bp->b_addr, XFS_LBSIZE(mp));
> > memset(bp->b_addr, 0, XFS_LBSIZE(mp));
> > + leaf_src = (xfs_attr_leafblock_t *)tmpbuffer;
> > + leaf_dst = bp->b_addr;
> >
> > /*
> > - * Copy basic information
> > + * Copy the on-disk header back into the destination buffer to ensure
> > + * all the information in the header that is not part of the incore
> > + * header structure is preserved.
> > */
> > - leaf_s = (xfs_attr_leafblock_t *)tmpbuffer;
> > - leaf_d = bp->b_addr;
> > - ichdr_s = *ichdr_d; /* struct copy */
> > - ichdr_d->firstused = XFS_LBSIZE(mp);
> > - ichdr_d->usedbytes = 0;
> > - ichdr_d->count = 0;
> > - ichdr_d->holes = 0;
> > - ichdr_d->freemap[0].base = xfs_attr3_leaf_hdr_size(leaf_s);
> > - ichdr_d->freemap[0].size = ichdr_d->firstused - ichdr_d->freemap[0].base;
> > + memcpy(bp->b_addr, tmpbuffer, xfs_attr3_leaf_hdr_size(leaf_src));
> > +
> > + /* Initialise the incore headers */
> > + ichdr_src = *ichdr_dst; /* struct copy */
> > + ichdr_dst->firstused = XFS_LBSIZE(mp);
> > + ichdr_dst->usedbytes = 0;
> > + ichdr_dst->count = 0;
> > + ichdr_dst->holes = 0;
> > + ichdr_dst->freemap[0].base = xfs_attr3_leaf_hdr_size(leaf_src);
> > + ichdr_dst->freemap[0].size = ichdr_dst->firstused -
> > + ichdr_dst->freemap[0].base;
> > +
> > +
> > + /* write the header back to initialise the underlying buffer */
> > + xfs_attr3_leaf_hdr_to_disk(leaf_dst, ichdr_dst);
>
> I can see that the memcpy is necessary but again I'm having a hard time
> understanding what field was messed up for moveents.
moveents wasn't messed up - it was all the non-incore headers that
were invalid (owner, uuid, blkno, etc) due to the zeroing of the tmp
buffer and then only writing the incore header back to it after
moveents. That's what the addition memcpy of the header fixes, and
then the xfs_attr3_leaf_hdr_to_disk() call updates all the fields
modified by initialisation so that before we start the in-core and
"on-disk" structures are identical.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
