On Thu, Jan 04, 2018 at 06:49:24PM -0800, Darrick J. Wong wrote:
> On Fri, Jan 05, 2018 at 12:40:53PM +1100, Dave Chinner wrote:
> > On Fri, Dec 22, 2017 at 04:43:41PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > >
> > > Add a couple of functions to the rmap btrees that will be used
> > > to cross-reference metadata against the rmapbt.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > ---
> > > fs/xfs/libxfs/xfs_rmap.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
> > > fs/xfs/libxfs/xfs_rmap.h | 5 ++++
> > > 2 files changed, 63 insertions(+)
> > >
> > >
> > > diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> > > index 50db920..ea78ec3 100644
> > > --- a/fs/xfs/libxfs/xfs_rmap.c
> > > +++ b/fs/xfs/libxfs/xfs_rmap.c
> > > @@ -2387,3 +2387,61 @@ xfs_rmap_compare(
> > > else
> > > return 0;
> > > }
> > > +
> > > +/* Is there a record covering a given extent? */
> > > +int
> > > +xfs_rmap_has_record(
> > > + struct xfs_btree_cur *cur,
> > > + xfs_agblock_t bno,
> > > + xfs_extlen_t len,
> > > + bool *exists)
> > > +{
> > > + union xfs_btree_irec low;
> > > + union xfs_btree_irec high;
> > > +
> > > + memset(&low, 0, sizeof(low));
> > > + low.r.rm_startblock = bno;
> > > + memset(&high, 0xFF, sizeof(high));
> > > + high.r.rm_startblock = bno + len - 1;
> > > +
> > > + return xfs_btree_has_record(cur, &low, &high, exists);
> > > +}
> > > +
> > > +/* Is there a record covering a given extent? */
> > > +int
> > > +xfs_rmap_record_exists(
> > > + struct xfs_btree_cur *cur,
> > > + xfs_agblock_t bno,
> > > + xfs_extlen_t len,
> > > + struct xfs_owner_info *oinfo,
> > > + bool *has_rmap)
> > > +{
> > > + uint64_t owner;
> > > + uint64_t offset;
> > > + unsigned int flags;
> > > + int stat;
> >
> > has_record to match the other code?
>
> Fixed.
>
> > > + struct xfs_rmap_irec irec;
> > > + int error;
> > > +
> > > + xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
> > > +
> > > + error = xfs_rmap_lookup_le(cur, bno, len, owner, offset, flags, &stat);
> > > + if (error)
> > > + return error;
> > > + if (!stat) {
> > > + *has_rmap = false;
> > > + return 0;
> > > + }
> > > +
> > > + error = xfs_rmap_get_rec(cur, &irec, &stat);
> > > + if (error)
> > > + return error;
> > > + if (!stat) {
> > > + *has_rmap = false;
> > > + return 0;
> > > + }
> > > +
> > > + *has_rmap = (irec.rm_owner == owner && irec.rm_startblock <= bno &&
> > > + irec.rm_startblock + irec.rm_blockcount >= bno + len);
> >
> > Ok, so this returns true only if the rmap record spans the entire
> > range we pass in. What does it mean if the rmap record only
> > partially spans the range passed in?
>
> For the current users (i.e. scrub) we require that the rmap completely
> overlap the queried range. If there's a gap anywhere (or mergeable
> records) this function returns false and the calling scrub function
> records a scrub xref failure.
>
> How about the following change to the comment above the function?
>
> "Is there a record for this owner completely covering a given physical
> extent? If so, *has_rmap will be set to true. If there is no record
> or the record only covers part of the range, we set *has_rmap to false.
> This function doesn't perform range lookups or offset checks, so it is
> not suitable for checking data fork blocks."
>
> & maybe a:
>
> ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
> XFS_RMAP_IS_BMBT_BLOCK(offset));
Yup, that explains what is going on a whole lot better :P
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
