Re: [PATCH v2 08/92] fs: new helper: simple_rename_timestamp

* Re: [PATCH v2 08/92] fs: new helper: simple_rename_timestamp
       [not found]   ` <20230705185812.579118-3-jlayton-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2023-07-06 10:27     ` Jan Kara
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kara @ 2023-07-06 10:27 UTC (permalink / raw)
  To: Jeff Layton
  Cc: jk-mnsaURCQ41sdnm+yROfE0A, arnd-r2nGTMty4D4,
	mpe-Gsx/Oe8HsFggBc27wqDAHg, npiggin-Re5JQEeQqe8AvxtiuMwx3w,
	christophe.leroy-2tlSp11Fh4xulxpn9UvDqw,
	hca-tEXmvtCZX7AybS5Ee8rs3A, gor-tEXmvtCZX7AybS5Ee8rs3A,
	agordeev-tEXmvtCZX7AybS5Ee8rs3A,
	borntraeger-tEXmvtCZX7AybS5Ee8rs3A, svens-tEXmvtCZX7AybS5Ee8rs3A,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	arve-z5hGa2qSFaRBDgjK7y7TUQ, tkjos-z5hGa2qSFaRBDgjK7y7TUQ,
	maco-z5hGa2qSFaRBDgjK7y7TUQ,
	joel-QYYGw3jwrUn5owFQY34kdNi2O/JbrIOy,
	brauner-DgEjT+Ai2ygdnm+yROfE0A, cmllamas-hpIqsD4AKlfQT0dZR+AlfA,
	surenb-hpIqsD4AKlfQT0dZR+AlfA,
	dennis.dalessandro-ntyVByD3zXaTtA8H5PvdGFaTQe2KTcn/,
	jgg-uk2M96/98Pc, leon-DgEjT+Ai2ygdnm+yROfE0A,
	bwarrum-tEXmvtCZX7AybS5Ee8rs3A, rituagar-tEXmvtCZX7AybS5Ee8rs3A,
	ericvh-DgEjT+Ai2ygdnm+yROfE0A, lucho-OnYtXJJ0/fesTnJN9+BGXg,
	asmadeus-9aU+PIKW3UvdtAWm4Da02A,
	linux_oss-kKOvUW9iAPtl57MIdRCFDg, dsterba-IBi9RG/b67k,
	dhowells-H+wXaHxf7aLQT0dZR+AlfA,
	marc.dionne-hRzEac23uH1Wk0Htik3J/w,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	raven-PKsaG3nR2I+sTnJN9+BGXg, luisbg-DgEjT+Ai2ygdnm+yROfE0A,
	salah.triki-Re5JQEeQqe8AvxtiuMwx3w,
	aivazian.tigran-Re5JQEeQqe8AvxtiuMwx3w,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, kees

On Wed 05-07-23 14:58:11, Jeff Layton wrote:
> A rename potentially involves updating 4 different inode timestamps. Add
> a function that handles the details sanely, and convert the libfs.c
> callers to use it.
> 
> Signed-off-by: Jeff Layton <jlayton-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>

								Honza

> ---
>  fs/libfs.c         | 36 +++++++++++++++++++++++++++---------
>  include/linux/fs.h |  2 ++
>  2 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index a7e56baf8bbd..9ee79668c909 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -692,6 +692,31 @@ int simple_rmdir(struct inode *dir, struct dentry *dentry)
>  }
>  EXPORT_SYMBOL(simple_rmdir);
>  
> +/**
> + * simple_rename_timestamp - update the various inode timestamps for rename
> + * @old_dir: old parent directory
> + * @old_dentry: dentry that is being renamed
> + * @new_dir: new parent directory
> + * @new_dentry: target for rename
> + *
> + * POSIX mandates that the old and new parent directories have their ctime and
> + * mtime updated, and that inodes of @old_dentry and @new_dentry (if any), have
> + * their ctime updated.
> + */
> +void simple_rename_timestamp(struct inode *old_dir, struct dentry *old_dentry,
> +			     struct inode *new_dir, struct dentry *new_dentry)
> +{
> +	struct inode *newino = d_inode(new_dentry);
> +
> +	old_dir->i_mtime = inode_set_ctime_current(old_dir);
> +	if (new_dir != old_dir)
> +		new_dir->i_mtime = inode_set_ctime_current(new_dir);
> +	inode_set_ctime_current(d_inode(old_dentry));
> +	if (newino)
> +		inode_set_ctime_current(newino);
> +}
> +EXPORT_SYMBOL_GPL(simple_rename_timestamp);
> +
>  int simple_rename_exchange(struct inode *old_dir, struct dentry *old_dentry,
>  			   struct inode *new_dir, struct dentry *new_dentry)
>  {
> @@ -707,11 +732,7 @@ int simple_rename_exchange(struct inode *old_dir, struct dentry *old_dentry,
>  			inc_nlink(old_dir);
>  		}
>  	}
> -	old_dir->i_ctime = old_dir->i_mtime =
> -	new_dir->i_ctime = new_dir->i_mtime =
> -	d_inode(old_dentry)->i_ctime =
> -	d_inode(new_dentry)->i_ctime = current_time(old_dir);
> -
> +	simple_rename_timestamp(old_dir, old_dentry, new_dir, new_dentry);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(simple_rename_exchange);
> @@ -720,7 +741,6 @@ int simple_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  		  struct dentry *old_dentry, struct inode *new_dir,
>  		  struct dentry *new_dentry, unsigned int flags)
>  {
> -	struct inode *inode = d_inode(old_dentry);
>  	int they_are_dirs = d_is_dir(old_dentry);
>  
>  	if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE))
> @@ -743,9 +763,7 @@ int simple_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  		inc_nlink(new_dir);
>  	}
>  
> -	old_dir->i_ctime = old_dir->i_mtime = new_dir->i_ctime =
> -		new_dir->i_mtime = inode->i_ctime = current_time(old_dir);
> -
> +	simple_rename_timestamp(old_dir, old_dentry, new_dir, new_dentry);
>  	return 0;
>  }
>  EXPORT_SYMBOL(simple_rename);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bdfbd11a5811..14e38bd900f1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2979,6 +2979,8 @@ extern int simple_open(struct inode *inode, struct file *file);
>  extern int simple_link(struct dentry *, struct inode *, struct dentry *);
>  extern int simple_unlink(struct inode *, struct dentry *);
>  extern int simple_rmdir(struct inode *, struct dentry *);
> +void simple_rename_timestamp(struct inode *old_dir, struct dentry *old_dentry,
> +			     struct inode *new_dir, struct dentry *new_dentry);
>  extern int simple_rename_exchange(struct inode *old_dir, struct dentry *old_dentry,
>  				  struct inode *new_dir, struct dentry *new_dentry);
>  extern int simple_rename(struct mnt_idmap *, struct inode *,
> -- 
> 2.41.0
> 
-- 
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 13+ messages in thread