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

[Brian Foster <bfoster@redhat.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs