On Mon, Jul 11, 2016 at 02:49:09PM -0400, Brian Foster wrote:
> On Thu, Jun 16, 2016 at 06:21:43PM -0700, Darrick J. Wong wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > 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 separate patch, so just the
> > addition operation can be focussed on here.
> >
> > v2: Update alloc function to handle non-shared file data. Isolate the
> > part that makes changes from the part that initializes the rmap
> > cursor; this will be useful for deferred updates.
> >
> > [darrick: handle owner offsets when adding rmaps]
> > [dchinner: remove remaining debug printk statements]
> > [darrick: move unwritten bit to rm_offset]
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_rmap.c | 225 +++++++++++++++++++++++++++++++++++++++-
> > fs/xfs/libxfs/xfs_rmap_btree.h | 1
> > fs/xfs/xfs_trace.h | 2
> > 3 files changed, 223 insertions(+), 5 deletions(-)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> > index 0e1721a..196e952 100644
> > --- a/fs/xfs/libxfs/xfs_rmap.c
> > +++ b/fs/xfs/libxfs/xfs_rmap.c
> > @@ -159,6 +159,218 @@ out_error:
> > return error;
> > }
> >
> > +/*
> > + * A mergeable rmap should have the same owner, cannot be unwritten, and
>
> Why can't it be unwritten? According to the code, it just looks like the
> unwritten state must match between extents..?
Correct. The comment needs to be updated.
/*
* A mergeable rmap must have the same owner and the same values for
* the unwritten, attr_fork, and bmbt flags. The startblock and
* offsets are checked separately.
*/
>
> > + * must be a bmbt rmap if we're asking about a bmbt rmap.
> > + */
> > +static bool
> > +xfs_rmap_is_mergeable(
> > + struct xfs_rmap_irec *irec,
> > + uint64_t owner,
> > + uint64_t offset,
> > + xfs_extlen_t len,
> > + unsigned int flags)
> > +{
>
> Also, why are we passing and not using offset and len? Is this modified
> later?
Actually... offset and len are unnecessary. len falls out in a later
patch, so I will eliminate both when I clean this up.
> One more comment nit below, otherwise looks good:
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> > + if (irec->rm_owner == XFS_RMAP_OWN_NULL)
> > + return false;
> > + if (irec->rm_owner != owner)
> > + return false;
> > + if ((flags & XFS_RMAP_UNWRITTEN) ^
> > + (irec->rm_flags & XFS_RMAP_UNWRITTEN))
> > + return false;
> > + if ((flags & XFS_RMAP_ATTR_FORK) ^
> > + (irec->rm_flags & XFS_RMAP_ATTR_FORK))
> > + return false;
> > + if ((flags & XFS_RMAP_BMBT_BLOCK) ^
> > + (irec->rm_flags & XFS_RMAP_BMBT_BLOCK))
> > + return false;
> > + return true;
> > +}
> > +
> > +/*
> > + * 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, offset] record. Flags are encoded in the high bits of the offset
> > + * field.
> > + */
> > +STATIC int
> > +__xfs_rmap_alloc(
> > + struct xfs_btree_cur *cur,
> > + xfs_agblock_t bno,
> > + xfs_extlen_t len,
> > + bool unwritten,
> > + struct xfs_owner_info *oinfo)
> > +{
> > + struct xfs_mount *mp = cur->bc_mp;
> > + struct xfs_rmap_irec ltrec;
> > + struct xfs_rmap_irec gtrec;
> > + int have_gt;
> > + int have_lt;
> > + int error = 0;
> > + int i;
> > + uint64_t owner;
> > + uint64_t offset;
> > + unsigned int flags = 0;
> > + bool ignore_off;
> > +
> > + xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
> > + ignore_off = XFS_RMAP_NON_INODE_OWNER(owner) ||
> > + (flags & XFS_RMAP_BMBT_BLOCK);
> > + if (unwritten)
> > + flags |= XFS_RMAP_UNWRITTEN;
> > + trace_xfs_rmap_alloc_extent(mp, cur->bc_private.a.agno, bno, len,
> > + unwritten, oinfo);
> > +
> > + /*
> > + * For the initial lookup, look for and exact match or the left-adjacent
>
> an
Noted. Thanks for catching these!
--D
>
> Brian
>
> > + * 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, offset, flags,
> > + &have_lt);
> > + if (error)
> > + goto out_error;
> > + XFS_WANT_CORRUPTED_GOTO(mp, have_lt == 1, out_error);
> > +
> > + error = xfs_rmap_get_rec(cur, <rec, &have_lt);
> > + if (error)
> > + goto out_error;
> > + XFS_WANT_CORRUPTED_GOTO(mp, have_lt == 1, out_error);
> > + trace_xfs_rmap_lookup_le_range_result(cur->bc_mp,
> > + cur->bc_private.a.agno, ltrec.rm_startblock,
> > + ltrec.rm_blockcount, ltrec.rm_owner,
> > + ltrec.rm_offset, ltrec.rm_flags);
> > +
> > + if (!xfs_rmap_is_mergeable(<rec, owner, offset, len, flags))
> > + have_lt = 0;
> > +
> > + XFS_WANT_CORRUPTED_GOTO(mp,
> > + have_lt == 0 ||
> > + ltrec.rm_startblock + ltrec.rm_blockcount <= bno, out_error);
> > +
> > + /*
> > + * Increment the cursor to see if we have a right-adjacent record to our
> > + * insertion point. This will give us the record for end block
> > + * contiguity tests.
> > + */
> > + error = xfs_btree_increment(cur, 0, &have_gt);
> > + if (error)
> > + goto out_error;
> > + if (have_gt) {
> > + error = xfs_rmap_get_rec(cur, >rec, &have_gt);
> > + if (error)
> > + goto out_error;
> > + XFS_WANT_CORRUPTED_GOTO(mp, have_gt == 1, out_error);
> > + XFS_WANT_CORRUPTED_GOTO(mp, bno + len <= gtrec.rm_startblock,
> > + out_error);
> > + trace_xfs_rmap_map_gtrec(cur->bc_mp,
> > + cur->bc_private.a.agno, gtrec.rm_startblock,
> > + gtrec.rm_blockcount, gtrec.rm_owner,
> > + gtrec.rm_offset, gtrec.rm_flags);
> > + if (!xfs_rmap_is_mergeable(>rec, owner, offset, len, flags))
> > + have_gt = 0;
> > + }
> > +
> > + /*
> > + * Note: cursor currently points one record to the right of ltrec, even
> > + * if there is no record in the tree to the right.
> > + */
> > + if (have_lt &&
> > + ltrec.rm_startblock + ltrec.rm_blockcount == bno &&
> > + (ignore_off || ltrec.rm_offset + ltrec.rm_blockcount == offset)) {
> > + /*
> > + * left edge contiguous, merge into left record.
> > + *
> > + * ltbno ltlen
> > + * orig: |ooooooooo|
> > + * adding: |aaaaaaaaa|
> > + * result: |rrrrrrrrrrrrrrrrrrr|
> > + * bno len
> > + */
> > + ltrec.rm_blockcount += len;
> > + if (have_gt &&
> > + bno + len == gtrec.rm_startblock &&
> > + (ignore_off || offset + len == gtrec.rm_offset) &&
> > + (unsigned long)ltrec.rm_blockcount + len +
> > + gtrec.rm_blockcount <= XFS_RMAP_LEN_MAX) {
> > + /*
> > + * right edge also contiguous, delete right record
> > + * and merge into left record.
> > + *
> > + * ltbno ltlen gtbno gtlen
> > + * orig: |ooooooooo| |ooooooooo|
> > + * adding: |aaaaaaaaa|
> > + * result: |rrrrrrrrrrrrrrrrrrrrrrrrrrrrr|
> > + */
> > + ltrec.rm_blockcount += gtrec.rm_blockcount;
> > + trace_xfs_rmapbt_delete(mp, cur->bc_private.a.agno,
> > + gtrec.rm_startblock,
> > + gtrec.rm_blockcount,
> > + gtrec.rm_owner,
> > + gtrec.rm_offset,
> > + gtrec.rm_flags);
> > + error = xfs_btree_delete(cur, &i);
> > + if (error)
> > + goto out_error;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, out_error);
> > + }
> > +
> > + /* point the cursor back to the left record and update */
> > + error = xfs_btree_decrement(cur, 0, &have_gt);
> > + if (error)
> > + goto out_error;
> > + error = xfs_rmap_update(cur, <rec);
> > + if (error)
> > + goto out_error;
> > + } else if (have_gt &&
> > + bno + len == gtrec.rm_startblock &&
> > + (ignore_off || offset + len == gtrec.rm_offset)) {
> > + /*
> > + * right edge contiguous, merge into right record.
> > + *
> > + * gtbno gtlen
> > + * Orig: |ooooooooo|
> > + * adding: |aaaaaaaaa|
> > + * Result: |rrrrrrrrrrrrrrrrrrr|
> > + * bno len
> > + */
> > + gtrec.rm_startblock = bno;
> > + gtrec.rm_blockcount += len;
> > + if (!ignore_off)
> > + gtrec.rm_offset = offset;
> > + error = xfs_rmap_update(cur, >rec);
> > + if (error)
> > + goto out_error;
> > + } else {
> > + /*
> > + * no contiguous edge with identical owner, insert
> > + * new record at current cursor position.
> > + */
> > + cur->bc_rec.r.rm_startblock = bno;
> > + cur->bc_rec.r.rm_blockcount = len;
> > + cur->bc_rec.r.rm_owner = owner;
> > + cur->bc_rec.r.rm_offset = offset;
> > + cur->bc_rec.r.rm_flags = flags;
> > + trace_xfs_rmapbt_insert(mp, cur->bc_private.a.agno, bno, len,
> > + owner, offset, flags);
> > + error = xfs_btree_insert(cur, &i);
> > + if (error)
> > + goto out_error;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, out_error);
> > + }
> > +
> > + trace_xfs_rmap_alloc_extent_done(mp, cur->bc_private.a.agno, bno, len,
> > + unwritten, oinfo);
> > +out_error:
> > + if (error)
> > + trace_xfs_rmap_alloc_extent_error(mp, cur->bc_private.a.agno,
> > + bno, len, unwritten, oinfo);
> > + return error;
> > +}
> > +
> > +/*
> > + * Add a reference to an extent in the rmap btree.
> > + */
> > int
> > xfs_rmap_alloc(
> > struct xfs_trans *tp,
> > @@ -169,19 +381,22 @@ xfs_rmap_alloc(
> > struct xfs_owner_info *oinfo)
> > {
> > struct xfs_mount *mp = tp->t_mountp;
> > - int error = 0;
> > + struct xfs_btree_cur *cur;
> > + int error;
> >
> > if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
> > return 0;
> >
> > - trace_xfs_rmap_alloc_extent(mp, agno, bno, len, false, oinfo);
> > - if (1)
> > + cur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno);
> > + error = __xfs_rmap_alloc(cur, bno, len, false, oinfo);
> > + if (error)
> > goto out_error;
> > - trace_xfs_rmap_alloc_extent_done(mp, agno, bno, len, false, oinfo);
> > +
> > + xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
> > return 0;
> >
> > out_error:
> > - trace_xfs_rmap_alloc_extent_error(mp, agno, bno, len, false, oinfo);
> > + xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
> > return error;
> > }
> >
> > diff --git a/fs/xfs/libxfs/xfs_rmap_btree.h b/fs/xfs/libxfs/xfs_rmap_btree.h
> > index e926c6e..9d92da5 100644
> > --- a/fs/xfs/libxfs/xfs_rmap_btree.h
> > +++ b/fs/xfs/libxfs/xfs_rmap_btree.h
> > @@ -67,6 +67,7 @@ int xfs_rmap_lookup_eq(struct xfs_btree_cur *cur, xfs_agblock_t bno,
> > int xfs_rmap_get_rec(struct xfs_btree_cur *cur, struct xfs_rmap_irec *irec,
> > int *stat);
> >
> > +/* functions for updating the rmapbt for bmbt blocks and AG btree blocks */
> > int xfs_rmap_alloc(struct xfs_trans *tp, struct xfs_buf *agbp,
> > xfs_agnumber_t agno, xfs_agblock_t bno, xfs_extlen_t len,
> > struct xfs_owner_info *oinfo);
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 6daafaf..3ebceb0 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -2549,6 +2549,8 @@ DEFINE_RMAPBT_EVENT(xfs_rmapbt_delete);
> > DEFINE_AG_ERROR_EVENT(xfs_rmapbt_insert_error);
> > DEFINE_AG_ERROR_EVENT(xfs_rmapbt_delete_error);
> > DEFINE_AG_ERROR_EVENT(xfs_rmapbt_update_error);
> > +DEFINE_RMAPBT_EVENT(xfs_rmap_lookup_le_range_result);
> > +DEFINE_RMAPBT_EVENT(xfs_rmap_map_gtrec);
> >
> > #endif /* _TRACE_XFS_H */
> >
> >
> > _______________________________________________
> > xfs mailing list
> > xfs@xxxxxxxxxxx
> > http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
