[cc amir since he piped up about the v1 patch] On Fri, Feb 03, 2017 at 10:57:15AM +0100, Christoph Hellwig wrote: > We're changing both metadata and data, so we need to update the > timestamps for clone operations. Dedupe on the other hand does > not change file data, and only changes invisible metadata so the > timestamps should not be updated. > > This follows existing btrfs behavior. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > fs/xfs/xfs_reflink.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c > index d5a2cf2b469b..199ce0100bc6 100644 > --- a/fs/xfs/xfs_reflink.c > +++ b/fs/xfs/xfs_reflink.c > @@ -961,13 +961,15 @@ STATIC int > xfs_reflink_update_dest( > struct xfs_inode *dest, > xfs_off_t newlen, > - xfs_extlen_t cowextsize) > + xfs_extlen_t cowextsize, > + bool is_dedupe) > { > struct xfs_mount *mp = dest->i_mount; > struct xfs_trans *tp; > int error; > > - if (newlen <= i_size_read(VFS_I(dest)) && cowextsize == 0) > + if (is_dedupe && > + newlen <= i_size_read(VFS_I(dest)) && cowextsize == 0 && is_dedupe) > return 0; Redundant is_dedupe tests here. I also wonder if we really /want/ to emulate the existing btrfs behavior, which seems to be: reflink: update mtime & ctime dedupe: do not update mtime or ctime In particular, dedupe changes the inode metadata, which should qualify for a ctime update, right? --D > error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); > @@ -988,6 +990,10 @@ xfs_reflink_update_dest( > dest->i_d.di_flags2 |= XFS_DIFLAG2_COWEXTSIZE; > } > > + if (!is_dedupe) { > + xfs_trans_ichgtime(tp, dest, > + XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG); > + } > xfs_trans_log_inode(tp, dest, XFS_ILOG_CORE); > > error = xfs_trans_commit(tp); > @@ -1301,7 +1307,8 @@ xfs_reflink_remap_range( > !(dest->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE)) > cowextsize = src->i_d.di_cowextsize; > > - ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize); > + ret = xfs_reflink_update_dest(dest, pos_out + len, cowextsize, > + is_dedupe); > > out_unlock: > xfs_iunlock(src, XFS_MMAPLOCK_EXCL); > -- > 2.11.0 > > -- > 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