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

Dave Chinner
2015-07-10T00:39:26Z

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