On Thu, 2023-07-06 at 12:39 +0200, Jan Kara wrote: > On Wed 05-07-23 15:00:37, Jeff Layton wrote: > > A rename potentially involves updating 4 different inode timestamps. > > Convert to the new simple_rename_timestamp helper function. > > > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/exfat/namei.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c > > index d9b46fa36bff..e91022ff80ef 100644 > > --- a/fs/exfat/namei.c > > +++ b/fs/exfat/namei.c > > @@ -1312,8 +1312,8 @@ static int exfat_rename(struct mnt_idmap *idmap, > > goto unlock; > > > > inode_inc_iversion(new_dir); > > - new_dir->i_ctime = new_dir->i_mtime = new_dir->i_atime = > > - EXFAT_I(new_dir)->i_crtime = current_time(new_dir); > > + simple_rename_timestamp(old_dir, old_dentry, new_dir, new_dentry); > > + EXFAT_I(new_dir)->i_crtime = current_time(new_dir); > > Hum, you loose atime update with this. Not that it would make sense to have > it but it would probably deserve a comment in the changelog. > > Also why you use current_time(new_dir) here instead of say inode->i_ctime? > I think the atime update there is a mistake. A rename is not a "read" operation. I'll note it in the changelog. The i_crtime in exfat is the creation time (aka btime). I don't think it matters much which source that ultimately comes from, but now I'm wondering why it gets set here at all. Does exfat create a new inode during a rename? If not, then the i_crtime updates here should probably be removed. > > exfat_truncate_atime(&new_dir->i_atime); > > if (IS_DIRSYNC(new_dir)) > > exfat_sync_inode(new_dir); > > @@ -1336,7 +1336,6 @@ static int exfat_rename(struct mnt_idmap *idmap, > > } > > > > inode_inc_iversion(old_dir); > > - old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir); > > if (IS_DIRSYNC(old_dir)) > > exfat_sync_inode(old_dir); > > else > > Also there is: > > new_inode->i_ctime = EXFAT_I(new_inode)->i_crtime = > current_time(new_inode); > > in exfat_rename() from which you can remove the ctime update? > Yeah, that should be removed too. I'll fix that in my tree. The i_crtime update here looks pretty suspicious too, fwiw. Thanks! -- Jeff Layton <jlayton@xxxxxxxxxx>
