| Message ID | 151398982164.18741.14153975836991113575.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Dec 22, 2017 at 04:43:41PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > 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@oracle.com> > --- > 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? > + 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? Cheers, Dave.
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@oracle.com> > > > > 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@oracle.com> > > --- > > 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)); --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@oracle.com> > > > > > > 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@oracle.com> > > > --- > > > 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.
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; + 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); + return 0; +} diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h index 0fcd5b1..380e53b 100644 --- a/fs/xfs/libxfs/xfs_rmap.h +++ b/fs/xfs/libxfs/xfs_rmap.h @@ -233,5 +233,10 @@ int xfs_rmap_compare(const struct xfs_rmap_irec *a, union xfs_btree_rec; int xfs_rmap_btrec_to_irec(union xfs_btree_rec *rec, struct xfs_rmap_irec *irec); +int xfs_rmap_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno, + xfs_extlen_t len, bool *exists); +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); #endif /* __XFS_RMAP_H__ */