On Thu, Jun 25, 2020 at 08:28:18AM -0400, Brian Foster wrote:
> On Wed, Jun 24, 2020 at 06:18:18PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > If the source and destination map are identical, we can skip the remap
> > step to save some time.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_reflink.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> >
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index 72de7179399d..f1156f121b7d 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -1031,6 +1031,23 @@ xfs_reflink_remap_extent(
> >
> > trace_xfs_reflink_remap_extent_dest(ip, &smap);
> >
> > + /*
> > + * Two extents mapped to the same physical block must not have
> > + * different states; that's filesystem corruption. Move on to the next
> > + * extent if they're both holes or both the same physical extent.
> > + */
> > + if (dmap->br_startblock == smap.br_startblock) {
> > + ASSERT(dmap->br_startblock == smap.br_startblock);
>
> That assert duplicates the logic in the if statement. Was this intended
> to be the length check I asked for? If so it looks like that was added
> previously so perhaps this can just drop off. With that fixed up:
Yep, I got hopelessly distracted while trying to figure out why Dave's
inode flush series keep crashing here. Will fix. :/
--D
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> > + if (dmap->br_state != smap.br_state)
> > + error = -EFSCORRUPTED;
> > + goto out_cancel;
> > + }
> > +
> > + /* If both extents are unwritten, leave them alone. */
> > + if (dmap->br_state == XFS_EXT_UNWRITTEN &&
> > + smap.br_state == XFS_EXT_UNWRITTEN)
> > + goto out_cancel;
> > +
> > /* No reflinking if the AG of the dest mapping is low on space. */
> > if (dmap_written) {
> > error = xfs_reflink_ag_has_free_space(mp,
> >
>
