On Wed, Mar 28, 2018 at 10:03:27AM +1100, Dave Chinner wrote:
> On Mon, Mar 26, 2018 at 04:56:13PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Add a couple of functions to the reverse mapping btree that will be used
> > to repair the rmapbt.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Not exactly sure how/what these will be used for yet, so it's hard
> to say if they are correct or not. :P
>
> Couple of things, though.
>
> > @@ -2454,3 +2482,54 @@ xfs_rmap_record_exists(
> > irec.rm_startblock + irec.rm_blockcount >= bno + len);
> > return 0;
> > }
> > +
> > +struct xfs_rmap_has_other_keys {
> > + uint64_t owner;
> > + uint64_t offset;
> > + bool *has_rmap;
>
> Seems very strange this is a pointer, given we are passing the
> structure.
>
> > + unsigned int flags;
> > +};
>
> struct xfs_rmap_key_state
Ok.
> > +/* For each rmap given, figure out if it doesn't match the key we want. */
> > +STATIC int
> > +xfs_rmap_has_other_keys_helper(
> > + struct xfs_btree_cur *cur,
> > + struct xfs_rmap_irec *rec,
> > + void *priv)
>
> And this can become just xfs_rmap_has_other_keys()
Uh... did you mean (struct xfs_rmap_key_state *) ?
The awkward void *priv argument + type conversion is there because
xfs_rmap_query_range takes a function with a void pointer as its third
parameter, and changing it here causes gcc complaints.
> > +{
> > + struct xfs_rmap_has_other_keys *rhok = priv;
> > +
> > + if (rhok->owner == rec->rm_owner && rhok->offset == rec->rm_offset &&
> > + ((rhok->flags & rec->rm_flags) & XFS_RMAP_KEY_FLAGS) == rhok->flags)
> > + return 0;
> > + *rhok->has_rmap = true;
> > + return XFS_BTREE_QUERY_RANGE_ABORT;
> > +}
> > +
> > +/*
> > + * Given an extent and some owner info, can we find records overlapping
> > + * the extent whose owner info does not match the given owner?
> > + */
> > +int
> > +xfs_rmap_has_other_keys(
> > + struct xfs_btree_cur *cur,
> > + xfs_agblock_t bno,
> > + xfs_extlen_t len,
> > + struct xfs_owner_info *oinfo,
> > + bool *has_rmap)
> > +{
> > + struct xfs_rmap_irec low = {0};
> > + struct xfs_rmap_irec high;
> > + struct xfs_rmap_has_other_keys rhok;
> > +
> > + xfs_owner_info_unpack(oinfo, &rhok.owner, &rhok.offset, &rhok.flags);
> > + *has_rmap = false;
> > + rhok.has_rmap = has_rmap;
>
> Oh, you're embedding another bool variable in it. I think it would
> be better to copy in/out of the internal structure rather than
> pass a pointer of unknown scope around....
>
> rhok.has_rmap = *has_rmap;
Fair 'enough.
> > +
> > + low.rm_startblock = bno;
> > + memset(&high, 0xFF, sizeof(high));
> > + high.rm_startblock = bno + len - 1;
> > +
> > + return xfs_rmap_query_range(cur, &low, &high,
> > + xfs_rmap_has_other_keys_helper, &rhok);
>
> error = xfs_rmap_query_range(..., &rhok)
> *has_rmap = rhok.has_rmap;
> return error;
Done.
--D
> 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
--
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
