On Thu, Aug 29, 2019 at 09:01:18AM -0700, Darrick J. Wong wrote: > > > irec->rm_owner = be64_to_cpu(rec->rmap.rm_owner); > > > diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h > > > index 0c2c3cb73429..abe633403fd1 100644 > > > --- a/fs/xfs/libxfs/xfs_rmap.h > > > +++ b/fs/xfs/libxfs/xfs_rmap.h > > > @@ -68,6 +68,7 @@ xfs_rmap_irec_offset_unpack( > > > if (offset & ~(XFS_RMAP_OFF_MASK | XFS_RMAP_OFF_FLAGS)) > > > return -EFSCORRUPTED; > > > irec->rm_offset = XFS_RMAP_OFF(offset); > > > + irec->rm_flags = 0; > > > > The change looks sensible-ish. But why do we even have a separate > > xfs_rmap_irec_offset_unpack with a single caller nd out of the > > way in a header? Wouldn't it make sense to just merge the two > > functions? > > xfs_repair uses libxfs_rmap_irec_offset_unpack, which is why it's a > separate function. True. Athough repair would also benefit a lot from a version of xfs_rmap_btrec_to_irec that just takes a bmbt_key instead, but that is for another time.