Re: [PATCH 15/20] xfs: add an extent to the rmap btree

[Dave Chinner <david@fromorbit.com>]

On Thu, Jun 25, 2015 at 12:41:23PM -0400, Brian Foster wrote:
> On Wed, Jun 03, 2015 at 04:04:52PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Now all the btree, free space and transaction infrastructure is in
> > place, we can finally add the code to insert reverse mappings to the
> > rmap btree. Freeing will be done in a spearate patch, so just the
> > addition operation can be focussed on here.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_rmap.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 138 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> > index 38a92a1..c1e5d23 100644
> > --- a/fs/xfs/libxfs/xfs_rmap.c
> > +++ b/fs/xfs/libxfs/xfs_rmap.c
> > @@ -120,6 +120,18 @@ out_error:
> >  	return error;
> >  }
> >  
> > +/*
> > + * When we allocate a new block, the first thing we do is add a reference to the
> > + * extent in the rmap btree. This takes the form of a [agbno, length, owner]
> > + * record.  Newly inserted extents should never overlap with an existing extent
> > + * in the rmap btree. Hence the insertion is a relatively trivial exercise,
> > + * involving checking for adjacent records and merging if the new extent is
> > + * contiguous and has the same owner.
> > + *
> > + * Note that we have no MAXEXTLEN limits here when merging as the length in the
> > + * record has the full 32 bits available and hence a single record can track the
> > + * entire space in the AG.
> > + */
> >  int
> >  xfs_rmap_alloc(
> >  	struct xfs_trans	*tp,
> > @@ -130,18 +142,143 @@ xfs_rmap_alloc(
> >  	uint64_t		owner)
> >  {
> >  	struct xfs_mount	*mp = tp->t_mountp;
> > +	struct xfs_btree_cur	*cur;
> > +	struct xfs_rmap_irec	ltrec;
> > +	struct xfs_rmap_irec	gtrec;
> > +	int			have_gt;
> >  	int			error = 0;
> > +	int			i;
> >  
> >  	if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
> >  		return 0;
> >  
> >  	trace_xfs_rmap_alloc_extent(mp, agno, bno, len, owner);
> > -	if (1)
> > +	cur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno);
> > +
> > +	/*
> > +	 * For the initial lookup, look for and exact match or the left-adjacent
> > +	 * record for our insertion point. This will also give us the record for
> > +	 * start block contiguity tests.
> > +	 */
> > +	error = xfs_rmap_lookup_le(cur, bno, len, owner, &i);
> > +	if (error)
> > +		goto out_error;
> > +	XFS_WANT_CORRUPTED_GOTO(mp, i == 1, out_error);
> > +
> 
> Are we intentionally relying on the fact that a left record will always
> exist due to the static metadata blocks that start the AG? If so, I'd
> just suggest to note that in the comment above.

Yes. I thought there was a note somewhere about this, but I'll add a
new one...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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