On Wed, Jul 13, 2016 at 02:27:55PM -0400, Brian Foster wrote:
> On Thu, Jun 16, 2016 at 06:21:55PM -0700, Darrick J. Wong wrote:
> > Provide a function to convert an unwritten extent to a real one and
> > vice versa.
> >
> > v2: Move unwritten bit to rm_offset.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
>
> Just a few nits below. Those aside and with Darrick's bc_rec.b ->
> bc_rec.r fix:
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> > fs/xfs/libxfs/xfs_rmap.c | 442 ++++++++++++++++++++++++++++++++++++++++++++++
> > fs/xfs/xfs_trace.h | 6 +
> > 2 files changed, 448 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> > index 1043c63..53ba14e 100644
> > --- a/fs/xfs/libxfs/xfs_rmap.c
> > +++ b/fs/xfs/libxfs/xfs_rmap.c
> > @@ -610,6 +610,448 @@ out_error:
> > return error;
> > }
> >
> > +#define RMAP_LEFT_CONTIG (1 << 0)
> > +#define RMAP_RIGHT_CONTIG (1 << 1)
> > +#define RMAP_LEFT_FILLING (1 << 2)
> > +#define RMAP_RIGHT_FILLING (1 << 3)
> > +#define RMAP_LEFT_VALID (1 << 6)
> > +#define RMAP_RIGHT_VALID (1 << 7)
> > +
> > +#define LEFT r[0]
> > +#define RIGHT r[1]
> > +#define PREV r[2]
> > +#define NEW r[3]
> > +
> > +/*
> > + * Convert an unwritten extent to a real extent or vice versa.
> > + * Does not handle overlapping extents.
> > + */
> > +STATIC int
> > +__xfs_rmap_convert(
> > + struct xfs_btree_cur *cur,
> > + xfs_agblock_t bno,
> > + xfs_extlen_t len,
> > + bool unwritten,
> > + struct xfs_owner_info *oinfo)
> > +{
> ...
> > +
> > + /*
> > + * For the initial lookup, look for and exact match or the left-adjacent
>
> Typo: an
>
> > + * 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, oldext, &i);
> > + if (error)
> > + goto done;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> > +
> ...
> > +
> > + /*
> > + * Switch out based on the FILLING and CONTIG state bits.
> > + */
> > + switch (state & (RMAP_LEFT_FILLING | RMAP_LEFT_CONTIG |
> > + RMAP_RIGHT_FILLING | RMAP_RIGHT_CONTIG)) {
> ...
> > + case RMAP_LEFT_FILLING | RMAP_RIGHT_FILLING | RMAP_RIGHT_CONTIG:
> > + /*
> > + * Setting all of a previous oldext extent to newext.
> > + * The right neighbor is contiguous, the left is not.
> > + */
> > + error = xfs_btree_increment(cur, 0, &i);
> > + if (error)
> > + goto done;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> > + trace_xfs_rmapbt_delete(mp, cur->bc_private.a.agno,
> > + RIGHT.rm_startblock, RIGHT.rm_blockcount,
> > + RIGHT.rm_owner, RIGHT.rm_offset,
> > + RIGHT.rm_flags);
> > + error = xfs_btree_delete(cur, &i);
> > + if (error)
> > + goto done;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> > + error = xfs_btree_decrement(cur, 0, &i);
> > + if (error)
> > + goto done;
> > + XFS_WANT_CORRUPTED_GOTO(mp, i == 1, done);
> > + NEW.rm_startblock = bno;
> > + NEW.rm_owner = owner;
> > + NEW.rm_offset = offset;
>
> NEW = PREV ?
>
> > + NEW.rm_blockcount = len + RIGHT.rm_blockcount;
> > + NEW.rm_flags = newext;
> > + error = xfs_rmap_update(cur, &NEW);
> > + if (error)
> > + goto done;
> > + break;
> > +
> ...
> > struct xfs_rmapbt_query_range_info {
> > xfs_rmapbt_query_range_fn fn;
> > void *priv;
> > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > index 3ebceb0..6466adc 100644
> > --- a/fs/xfs/xfs_trace.h
> > +++ b/fs/xfs/xfs_trace.h
> > @@ -2497,6 +2497,10 @@ DEFINE_RMAP_EVENT(xfs_rmap_free_extent_error);
> > DEFINE_RMAP_EVENT(xfs_rmap_alloc_extent);
> > DEFINE_RMAP_EVENT(xfs_rmap_alloc_extent_done);
> > DEFINE_RMAP_EVENT(xfs_rmap_alloc_extent_error);
> > +DEFINE_RMAP_EVENT(xfs_rmap_convert);
> > +DEFINE_RMAP_EVENT(xfs_rmap_convert_done);
> > +DEFINE_AG_ERROR_EVENT(xfs_rmap_convert_error);
> > +DEFINE_AG_ERROR_EVENT(xfs_rmap_convert_state);
> >
> > DECLARE_EVENT_CLASS(xfs_rmapbt_class,
> > TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno,
> > @@ -2551,6 +2555,8 @@ 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);
> > +DEFINE_RMAPBT_EVENT(xfs_rmap_convert_gtrec);
> > +DEFINE_RMAPBT_EVENT(xfs_rmap_find_left_neighbor_result);
>
> xfs_rmap_convert_ltrec ?
Originally there was a xfs_rmap_convert_ltrec and a xfs_map_convert_ltrec.
Then I had to create a real "find left extent" helper function for the
reflink versions of map/convert, and that became
xfs_rmap_find_left_neighbor*, with its own tracepoint.
It seemed silly to have different tracepoints for "here's what I found when
I went looking for a left-adjacent extent", so the non-reflink versions of
map/convert simply started (ab)using the xfs_rmap_find_left_neighbor_result
tracepoint, even though the non-shared versions open-code btree cursor
manipulation without doing a lookup.
I could refactor the whole mess to have functions to find the left and right
neighbors in shared and not-shared mode, but I find it easier to keep track
of the cursor manipulation if they all stay in one function.
Oh. Or I could just change the xfs_rmap_*_gtrec tracepoints into
xfs_rmap_find_right_neighbor_result.
Yeah, I'll do that since we already have a trace point at the top of the
function so we already know what we're doing.
--D
>
> Brian
>
> >
> > #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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
