xfs
[Top] [All Lists]

Re: [PATCH 14/13] xfs: swap leaf buffer into path struct atomically duri

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 14/13] xfs: swap leaf buffer into path struct atomically during path shift
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 17 Aug 2015 18:19:53 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20150817213413.GC714@dastard>
References: <1439233309-19959-1-git-send-email-bfoster@xxxxxxxxxx> <1439830072-61117-1-git-send-email-bfoster@xxxxxxxxxx> <20150817213413.GC714@dastard>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, Aug 18, 2015 at 07:34:13AM +1000, Dave Chinner wrote:
> On Mon, Aug 17, 2015 at 12:47:52PM -0400, Brian Foster wrote:
...
> > 
> >  fs/xfs/libxfs/xfs_da_btree.c | 25 ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> > index 3264d81..04a3765 100644
> > --- a/fs/xfs/libxfs/xfs_da_btree.c
> > +++ b/fs/xfs/libxfs/xfs_da_btree.c
> > @@ -1822,6 +1822,7 @@ xfs_da3_path_shift(
> >     struct xfs_da_args      *args;
> >     struct xfs_da_node_entry *btree;
> >     struct xfs_da3_icnode_hdr nodehdr;
> > +   struct xfs_buf          *bp;
> >     xfs_dablk_t             blkno = 0;
> >     int                     level;
> >     int                     error;
> > @@ -1865,21 +1866,27 @@ xfs_da3_path_shift(
> >      * same depth we were at originally.
> >      */
> >     for (blk++, level++; level < path->active; blk++, level++) {
> > +           struct xfs_buf  **bpp = &blk->bp;
> > +
> 
> What do we need this for? The new code is:
> 
> >             /*
> > +            * Read the next child block into a local buffer.
> >              */
> > +           error = xfs_da3_node_read(args->trans, dp, blkno, -1, &bp,
> > +                                     args->whichfork);
> > +           if (error)
> > +                   return error;
> >  
> >             /*
> > +            * Release the old block (if it's dirty, the trans doesn't
> > +            * actually let go) and swap the local buffer into the path
> > +            * structure. This ensures failure of the above read doesn't set
> > +            * a NULL buffer in an active slot in the path.
> >              */
> > +           if (release)
> > +                   xfs_trans_brelse(args->trans, blk->bp);
> >             blk->blkno = blkno;
> > +           *bpp = bp;
> 
> And this can simply be:
> 
>               blk->bp = bp;
> 
> so I don't think *bpp is necessary at all.
> 

Yeah, that's a thinko. v2 incoming...

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx

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