[PATCH 16/79] btrfs: switch to new ctime accessors

[PATCH 16/79] btrfs: switch to new ctime accessors

[Jeff Layton]

In later patches, we're going to change how the ctime.tv_nsec field is
utilized. Switch to using accessor functions instead of raw accesses of
inode->i_ctime.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/delayed-inode.c | 10 +++---
 fs/btrfs/file.c          | 24 +++++----------
 fs/btrfs/inode.c         | 66 ++++++++++++++++------------------------
 fs/btrfs/ioctl.c         |  2 +-
 fs/btrfs/reflink.c       |  7 ++---
 fs/btrfs/transaction.c   |  3 +-
 fs/btrfs/tree-log.c      |  4 +--
 fs/btrfs/xattr.c         |  4 +--
 8 files changed, 48 insertions(+), 72 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index c0a6a1784697..ae493a4dc206 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -1808,9 +1808,9 @@ static void fill_stack_inode_item(struct btrfs_trans_handle *trans,
 				      inode->i_mtime.tv_nsec);
 
 	btrfs_set_stack_timespec_sec(&inode_item->ctime,
-				     inode->i_ctime.tv_sec);
+				     inode_ctime_peek(inode).tv_sec);
 	btrfs_set_stack_timespec_nsec(&inode_item->ctime,
-				      inode->i_ctime.tv_nsec);
+				      inode_ctime_peek(inode).tv_nsec);
 
 	btrfs_set_stack_timespec_sec(&inode_item->otime,
 				     BTRFS_I(inode)->i_otime.tv_sec);
@@ -1861,8 +1861,10 @@ int btrfs_fill_inode(struct inode *inode, u32 *rdev)
 	inode->i_mtime.tv_sec = btrfs_stack_timespec_sec(&inode_item->mtime);
 	inode->i_mtime.tv_nsec = btrfs_stack_timespec_nsec(&inode_item->mtime);
 
-	inode->i_ctime.tv_sec = btrfs_stack_timespec_sec(&inode_item->ctime);
-	inode->i_ctime.tv_nsec = btrfs_stack_timespec_nsec(&inode_item->ctime);
+	inode_ctime_set_sec(inode,
+			    btrfs_stack_timespec_sec(&inode_item->ctime));
+	inode_ctime_set_nsec(inode,
+			     btrfs_stack_timespec_nsec(&inode_item->ctime));
 
 	BTRFS_I(inode)->i_otime.tv_sec =
 		btrfs_stack_timespec_sec(&inode_item->otime);
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fd03e689a6be..b4082b322b41 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1108,17 +1108,10 @@ void btrfs_check_nocow_unlock(struct btrfs_inode *inode)
 
 static void update_time_for_write(struct inode *inode)
 {
-	struct timespec64 now;
-
 	if (IS_NOCMTIME(inode))
 		return;
 
-	now = current_time(inode);
-	if (!timespec64_equal(&inode->i_mtime, &now))
-		inode->i_mtime = now;
-
-	if (!timespec64_equal(&inode->i_ctime, &now))
-		inode->i_ctime = now;
+	inode->i_mtime = inode_ctime_set_current(inode);
 
 	if (IS_I_VERSION(inode))
 		inode_inc_iversion(inode);
@@ -2460,8 +2453,9 @@ int btrfs_replace_file_extents(struct btrfs_inode *inode,
 		inode_inc_iversion(&inode->vfs_inode);
 
 		if (!extent_info || extent_info->update_times) {
-			inode->vfs_inode.i_mtime = current_time(&inode->vfs_inode);
-			inode->vfs_inode.i_ctime = inode->vfs_inode.i_mtime;
+			struct inode *vinode = &inode->vfs_inode;
+
+			vinode->i_mtime = inode_ctime_set_current(vinode);
 		}
 
 		ret = btrfs_update_inode(trans, root, inode);
@@ -2703,8 +2697,7 @@ static int btrfs_punch_hole(struct file *file, loff_t offset, loff_t len)
 
 	ASSERT(trans != NULL);
 	inode_inc_iversion(inode);
-	inode->i_mtime = current_time(inode);
-	inode->i_ctime = inode->i_mtime;
+	inode->i_mtime = inode_ctime_set_current(inode);
 	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 	updated_inode = true;
 	btrfs_end_transaction(trans);
@@ -2721,11 +2714,8 @@ static int btrfs_punch_hole(struct file *file, loff_t offset, loff_t len)
 		 * for detecting, at fsync time, if the inode isn't yet in the
 		 * log tree or it's there but not up to date.
 		 */
-		struct timespec64 now = current_time(inode);
-
 		inode_inc_iversion(inode);
-		inode->i_mtime = now;
-		inode->i_ctime = now;
+		inode->i_mtime = inode_ctime_set_current(inode);
 		trans = btrfs_start_transaction(root, 1);
 		if (IS_ERR(trans)) {
 			ret = PTR_ERR(trans);
@@ -2796,7 +2786,7 @@ static int btrfs_fallocate_update_isize(struct inode *inode,
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
 
-	inode->i_ctime = current_time(inode);
+	inode_ctime_set_current(inode);
 	i_size_write(inode, end);
 	btrfs_inode_safe_disk_i_size_write(BTRFS_I(inode), 0);
 	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 601fdc956484..4bbb6c6a7516 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3901,8 +3901,10 @@ static int btrfs_read_locked_inode(struct inode *inode,
 	inode->i_mtime.tv_sec = btrfs_timespec_sec(leaf, &inode_item->mtime);
 	inode->i_mtime.tv_nsec = btrfs_timespec_nsec(leaf, &inode_item->mtime);
 
-	inode->i_ctime.tv_sec = btrfs_timespec_sec(leaf, &inode_item->ctime);
-	inode->i_ctime.tv_nsec = btrfs_timespec_nsec(leaf, &inode_item->ctime);
+	inode_ctime_set_sec(inode,
+			    btrfs_timespec_sec(leaf, &inode_item->ctime));
+	inode_ctime_set_nsec(inode,
+			     btrfs_timespec_nsec(leaf, &inode_item->ctime));
 
 	BTRFS_I(inode)->i_otime.tv_sec =
 		btrfs_timespec_sec(leaf, &inode_item->otime);
@@ -4073,9 +4075,9 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
 				      inode->i_mtime.tv_nsec);
 
 	btrfs_set_token_timespec_sec(&token, &item->ctime,
-				     inode->i_ctime.tv_sec);
+				     inode_ctime_peek(inode).tv_sec);
 	btrfs_set_token_timespec_nsec(&token, &item->ctime,
-				      inode->i_ctime.tv_nsec);
+				      inode_ctime_peek(inode).tv_nsec);
 
 	btrfs_set_token_timespec_sec(&token, &item->otime,
 				     BTRFS_I(inode)->i_otime.tv_sec);
@@ -4273,9 +4275,8 @@ static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans,
 	btrfs_i_size_write(dir, dir->vfs_inode.i_size - name->len * 2);
 	inode_inc_iversion(&inode->vfs_inode);
 	inode_inc_iversion(&dir->vfs_inode);
-	inode->vfs_inode.i_ctime = current_time(&inode->vfs_inode);
-	dir->vfs_inode.i_mtime = inode->vfs_inode.i_ctime;
-	dir->vfs_inode.i_ctime = inode->vfs_inode.i_ctime;
+	inode_ctime_set_current(&inode->vfs_inode);
+	dir->vfs_inode.i_mtime = inode_ctime_set_current(&dir->vfs_inode);
 	ret = btrfs_update_inode(trans, root, dir);
 out:
 	return ret;
@@ -4448,8 +4449,7 @@ static int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
 
 	btrfs_i_size_write(dir, dir->vfs_inode.i_size - fname.disk_name.len * 2);
 	inode_inc_iversion(&dir->vfs_inode);
-	dir->vfs_inode.i_mtime = current_time(&dir->vfs_inode);
-	dir->vfs_inode.i_ctime = dir->vfs_inode.i_mtime;
+	dir->vfs_inode.i_mtime = inode_ctime_set_current(&dir->vfs_inode);
 	ret = btrfs_update_inode_fallback(trans, root, dir);
 	if (ret)
 		btrfs_abort_transaction(trans, ret);
@@ -5090,10 +5090,8 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr)
 	 */
 	if (newsize != oldsize) {
 		inode_inc_iversion(inode);
-		if (!(mask & (ATTR_CTIME | ATTR_MTIME))) {
-			inode->i_mtime = current_time(inode);
-			inode->i_ctime = inode->i_mtime;
-		}
+		if (!(mask & (ATTR_CTIME | ATTR_MTIME)))
+			inode->i_mtime = inode_ctime_set_current(inode);
 	}
 
 	if (newsize > oldsize) {
@@ -5736,9 +5734,7 @@ static struct inode *new_simple_dir(struct super_block *s,
 	inode->i_opflags &= ~IOP_XATTR;
 	inode->i_fop = &simple_dir_operations;
 	inode->i_mode = S_IFDIR | S_IRUGO | S_IWUSR | S_IXUGO;
-	inode->i_mtime = current_time(inode);
-	inode->i_atime = inode->i_mtime;
-	inode->i_ctime = inode->i_mtime;
+	inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
 	BTRFS_I(inode)->i_otime = inode->i_mtime;
 
 	return inode;
@@ -6075,7 +6071,7 @@ static int btrfs_update_time(struct inode *inode, struct timespec64 *now,
 	if (flags & S_VERSION)
 		dirty |= inode_maybe_inc_iversion(inode, dirty);
 	if (flags & S_CTIME)
-		inode->i_ctime = *now;
+		inode_ctime_set(inode, *now);
 	if (flags & S_MTIME)
 		inode->i_mtime = *now;
 	if (flags & S_ATIME)
@@ -6378,9 +6374,7 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
 		goto discard;
 	}
 
-	inode->i_mtime = current_time(inode);
-	inode->i_atime = inode->i_mtime;
-	inode->i_ctime = inode->i_mtime;
+	inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
 	BTRFS_I(inode)->i_otime = inode->i_mtime;
 
 	/*
@@ -6545,12 +6539,9 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 	 * log replay procedure is responsible for setting them to their correct
 	 * values (the ones it had when the fsync was done).
 	 */
-	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags)) {
-		struct timespec64 now = current_time(&parent_inode->vfs_inode);
+	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags))
+		parent_inode->vfs_inode.i_mtime = inode_ctime_set_current(&parent_inode->vfs_inode);
 
-		parent_inode->vfs_inode.i_mtime = now;
-		parent_inode->vfs_inode.i_ctime = now;
-	}
 	ret = btrfs_update_inode(trans, root, parent_inode);
 	if (ret)
 		btrfs_abort_transaction(trans, ret);
@@ -6690,7 +6681,7 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
 	BTRFS_I(inode)->dir_index = 0ULL;
 	inc_nlink(inode);
 	inode_inc_iversion(inode);
-	inode->i_ctime = current_time(inode);
+	inode_ctime_set_current(inode);
 	ihold(inode);
 	set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags);
 
@@ -8778,7 +8769,6 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 	struct btrfs_root *dest = BTRFS_I(new_dir)->root;
 	struct inode *new_inode = new_dentry->d_inode;
 	struct inode *old_inode = old_dentry->d_inode;
-	struct timespec64 ctime = current_time(old_inode);
 	struct btrfs_rename_ctx old_rename_ctx;
 	struct btrfs_rename_ctx new_rename_ctx;
 	u64 old_ino = btrfs_ino(BTRFS_I(old_inode));
@@ -8909,12 +8899,10 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 	inode_inc_iversion(new_dir);
 	inode_inc_iversion(old_inode);
 	inode_inc_iversion(new_inode);
-	old_dir->i_mtime = ctime;
-	old_dir->i_ctime = ctime;
-	new_dir->i_mtime = ctime;
-	new_dir->i_ctime = ctime;
-	old_inode->i_ctime = ctime;
-	new_inode->i_ctime = ctime;
+	old_dir->i_mtime = inode_ctime_set_current(old_dir);
+	new_dir->i_mtime = inode_ctime_set_current(new_dir);
+	inode_ctime_set_current(old_inode);
+	inode_ctime_set_current(new_inode);
 
 	if (old_dentry->d_parent != new_dentry->d_parent) {
 		btrfs_record_unlink_dir(trans, BTRFS_I(old_dir),
@@ -9178,11 +9166,9 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 	inode_inc_iversion(old_dir);
 	inode_inc_iversion(new_dir);
 	inode_inc_iversion(old_inode);
-	old_dir->i_mtime = current_time(old_dir);
-	old_dir->i_ctime = old_dir->i_mtime;
-	new_dir->i_mtime = old_dir->i_mtime;
-	new_dir->i_ctime = old_dir->i_mtime;
-	old_inode->i_ctime = old_dir->i_mtime;
+	old_dir->i_mtime = inode_ctime_set_current(old_dir);
+	new_dir->i_mtime = inode_ctime_set_current(new_dir);
+	inode_ctime_set_current(old_inode);
 
 	if (old_dentry->d_parent != new_dentry->d_parent)
 		btrfs_record_unlink_dir(trans, BTRFS_I(old_dir),
@@ -9204,7 +9190,7 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 
 	if (new_inode) {
 		inode_inc_iversion(new_inode);
-		new_inode->i_ctime = current_time(new_inode);
+		inode_ctime_set_current(new_inode);
 		if (unlikely(btrfs_ino(BTRFS_I(new_inode)) ==
 			     BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
 			ret = btrfs_unlink_subvol(trans, BTRFS_I(new_dir), new_dentry);
@@ -9744,7 +9730,7 @@ static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
 		*alloc_hint = ins.objectid + ins.offset;
 
 		inode_inc_iversion(inode);
-		inode->i_ctime = current_time(inode);
+		inode_ctime_set_current(inode);
 		BTRFS_I(inode)->flags |= BTRFS_INODE_PREALLOC;
 		if (!(mode & FALLOC_FL_KEEP_SIZE) &&
 		    (actual_len > inode->i_size) &&
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a895d105464b..3d50bd67ec85 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -384,7 +384,7 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
 	binode->flags = binode_flags;
 	btrfs_sync_inode_flags_to_i_flags(inode);
 	inode_inc_iversion(inode);
-	inode->i_ctime = current_time(inode);
+	inode_ctime_set_current(inode);
 	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 
  out_end_trans:
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 0474bbe39da7..3cf4716dea46 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -29,10 +29,9 @@ static int clone_finish_inode_update(struct btrfs_trans_handle *trans,
 	int ret;
 
 	inode_inc_iversion(inode);
-	if (!no_time_update) {
-		inode->i_mtime = current_time(inode);
-		inode->i_ctime = inode->i_mtime;
-	}
+	if (!no_time_update)
+		inode->i_mtime = inode_ctime_set_current(inode);
+
 	/*
 	 * We round up to the block size at eof when determining which
 	 * extents to clone above, but shouldn't round up the file size.
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index e7cfc992e02a..138f919646e2 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1831,8 +1831,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 
 	btrfs_i_size_write(BTRFS_I(parent_inode), parent_inode->i_size +
 						  fname.disk_name.len * 2);
-	parent_inode->i_mtime = current_time(parent_inode);
-	parent_inode->i_ctime = parent_inode->i_mtime;
+	parent_inode->i_mtime = inode_ctime_set_current(parent_inode);
 	ret = btrfs_update_inode_fallback(trans, parent_root, BTRFS_I(parent_inode));
 	if (ret) {
 		btrfs_abort_transaction(trans, ret);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 365a1cc0a3c3..c862d1450365 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -4148,9 +4148,9 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
 				      inode->i_mtime.tv_nsec);
 
 	btrfs_set_token_timespec_sec(&token, &item->ctime,
-				     inode->i_ctime.tv_sec);
+				     inode_ctime_peek(inode).tv_sec);
 	btrfs_set_token_timespec_nsec(&token, &item->ctime,
-				      inode->i_ctime.tv_nsec);
+				      inode_ctime_peek(inode).tv_nsec);
 
 	/*
 	 * We do not need to set the nbytes field, in fact during a fast fsync
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index fc4b20c2688a..a2d331436963 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -264,7 +264,7 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
 		goto out;
 
 	inode_inc_iversion(inode);
-	inode->i_ctime = current_time(inode);
+	inode_ctime_set_current(inode);
 	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 	if (ret)
 		btrfs_abort_transaction(trans, ret);
@@ -407,7 +407,7 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
 	ret = btrfs_set_prop(trans, inode, name, value, size, flags);
 	if (!ret) {
 		inode_inc_iversion(inode);
-		inode->i_ctime = current_time(inode);
+		inode_ctime_set_current(inode);
 		ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
 		if (ret)
 			btrfs_abort_transaction(trans, ret);
-- 
2.41.0

Re: [PATCH 01/79] fs: add ctime accessors infrastructure

[Jeff Layton]

On Thu, 2023-06-22 at 09:46 +0900, Damien Le Moal wrote:
> On 6/21/23 23:45, Jeff Layton wrote:
> > struct timespec64 has unused bits in the tv_nsec field that can be used
> > for other purposes. In future patches, we're going to change how the
> > inode->i_ctime is accessed in certain inodes in order to make use of
> > them. In order to do that safely though, we'll need to eradicate raw
> > accesses of the inode->i_ctime field from the kernel.
> > 
> > Add new accessor functions for the ctime that we can use to replace them.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> [...]
> 
> > +/**
> > + * inode_ctime_peek - fetch the current ctime from the inode
> > + * @inode: inode from which to fetch ctime
> > + *
> > + * Grab the current ctime from the inode and return it.
> > + */
> > +static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
> 
> To be consistent with inode_ctime_set(), why not call this one inode_ctime_get()

In later patches fetching the ctime for presentation may have side
effects on certain filesystems. Using "peek" here is a hint that we want
to avoid those side effects in these calls.

> ? Also, inode_set_ctime() & inode_get_ctime() may be a little more natural. But
> no strong opinion about that though.
> 

I like the consistency of the inode_ctime_* prefix. It makes it simpler
to find these calls when grepping, etc.

That said, my opinions on naming are pretty loosely-held, so if the
consensus is that the names should as you suggest, I'll go along with
it.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 16/79] btrfs: switch to new ctime accessors

[David Sterba]

On Wed, Jun 21, 2023 at 10:45:29AM -0400, Jeff Layton wrote:
> In later patches, we're going to change how the ctime.tv_nsec field is
> utilized. Switch to using accessor functions instead of raw accesses of
> inode->i_ctime.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/btrfs/delayed-inode.c | 10 +++---
>  fs/btrfs/file.c          | 24 +++++----------
>  fs/btrfs/inode.c         | 66 ++++++++++++++++------------------------
>  fs/btrfs/ioctl.c         |  2 +-
>  fs/btrfs/reflink.c       |  7 ++---
>  fs/btrfs/transaction.c   |  3 +-
>  fs/btrfs/tree-log.c      |  4 +--
>  fs/btrfs/xattr.c         |  4 +--
>  8 files changed, 48 insertions(+), 72 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index c0a6a1784697..ae493a4dc206 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -1808,9 +1808,9 @@ static void fill_stack_inode_item(struct btrfs_trans_handle *trans,
>  				      inode->i_mtime.tv_nsec);
>  
>  	btrfs_set_stack_timespec_sec(&inode_item->ctime,
> -				     inode->i_ctime.tv_sec);
> +				     inode_ctime_peek(inode).tv_sec);
>  	btrfs_set_stack_timespec_nsec(&inode_item->ctime,
> -				      inode->i_ctime.tv_nsec);
> +				      inode_ctime_peek(inode).tv_nsec);
>  
>  	btrfs_set_stack_timespec_sec(&inode_item->otime,
>  				     BTRFS_I(inode)->i_otime.tv_sec);
> @@ -1861,8 +1861,10 @@ int btrfs_fill_inode(struct inode *inode, u32 *rdev)
>  	inode->i_mtime.tv_sec = btrfs_stack_timespec_sec(&inode_item->mtime);
>  	inode->i_mtime.tv_nsec = btrfs_stack_timespec_nsec(&inode_item->mtime);
>  
> -	inode->i_ctime.tv_sec = btrfs_stack_timespec_sec(&inode_item->ctime);
> -	inode->i_ctime.tv_nsec = btrfs_stack_timespec_nsec(&inode_item->ctime);
> +	inode_ctime_set_sec(inode,
> +			    btrfs_stack_timespec_sec(&inode_item->ctime));
> +	inode_ctime_set_nsec(inode,
> +			     btrfs_stack_timespec_nsec(&inode_item->ctime));
>  
>  	BTRFS_I(inode)->i_otime.tv_sec =
>  		btrfs_stack_timespec_sec(&inode_item->otime);
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index fd03e689a6be..b4082b322b41 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1108,17 +1108,10 @@ void btrfs_check_nocow_unlock(struct btrfs_inode *inode)
>  
>  static void update_time_for_write(struct inode *inode)
>  {
> -	struct timespec64 now;
> -
>  	if (IS_NOCMTIME(inode))
>  		return;
>  
> -	now = current_time(inode);
> -	if (!timespec64_equal(&inode->i_mtime, &now))
> -		inode->i_mtime = now;
> -
> -	if (!timespec64_equal(&inode->i_ctime, &now))
> -		inode->i_ctime = now;
> +	inode->i_mtime = inode_ctime_set_current(inode);
>  
>  	if (IS_I_VERSION(inode))
>  		inode_inc_iversion(inode);
> @@ -2460,8 +2453,9 @@ int btrfs_replace_file_extents(struct btrfs_inode *inode,
>  		inode_inc_iversion(&inode->vfs_inode);
>  
>  		if (!extent_info || extent_info->update_times) {
> -			inode->vfs_inode.i_mtime = current_time(&inode->vfs_inode);
> -			inode->vfs_inode.i_ctime = inode->vfs_inode.i_mtime;
> +			struct inode *vinode = &inode->vfs_inode;

Please use vfs_inode for the variable name.

> +
> +			vinode->i_mtime = inode_ctime_set_current(vinode);
>  		}
>  
>  		ret = btrfs_update_inode(trans, root, inode);
> @@ -2703,8 +2697,7 @@ static int btrfs_punch_hole(struct file *file, loff_t offset, loff_t len)
>  
>  	ASSERT(trans != NULL);
>  	inode_inc_iversion(inode);
> -	inode->i_mtime = current_time(inode);
> -	inode->i_ctime = inode->i_mtime;
> +	inode->i_mtime = inode_ctime_set_current(inode);
>  	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
>  	updated_inode = true;
>  	btrfs_end_transaction(trans);
> @@ -2721,11 +2714,8 @@ static int btrfs_punch_hole(struct file *file, loff_t offset, loff_t len)
>  		 * for detecting, at fsync time, if the inode isn't yet in the
>  		 * log tree or it's there but not up to date.
>  		 */
> -		struct timespec64 now = current_time(inode);
> -
>  		inode_inc_iversion(inode);
> -		inode->i_mtime = now;
> -		inode->i_ctime = now;
> +		inode->i_mtime = inode_ctime_set_current(inode);
>  		trans = btrfs_start_transaction(root, 1);
>  		if (IS_ERR(trans)) {
>  			ret = PTR_ERR(trans);
> @@ -2796,7 +2786,7 @@ static int btrfs_fallocate_update_isize(struct inode *inode,
>  	if (IS_ERR(trans))
>  		return PTR_ERR(trans);
>  
> -	inode->i_ctime = current_time(inode);
> +	inode_ctime_set_current(inode);
>  	i_size_write(inode, end);
>  	btrfs_inode_safe_disk_i_size_write(BTRFS_I(inode), 0);
>  	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 601fdc956484..4bbb6c6a7516 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3901,8 +3901,10 @@ static int btrfs_read_locked_inode(struct inode *inode,
>  	inode->i_mtime.tv_sec = btrfs_timespec_sec(leaf, &inode_item->mtime);
>  	inode->i_mtime.tv_nsec = btrfs_timespec_nsec(leaf, &inode_item->mtime);
>  
> -	inode->i_ctime.tv_sec = btrfs_timespec_sec(leaf, &inode_item->ctime);
> -	inode->i_ctime.tv_nsec = btrfs_timespec_nsec(leaf, &inode_item->ctime);
> +	inode_ctime_set_sec(inode,
> +			    btrfs_timespec_sec(leaf, &inode_item->ctime));
> +	inode_ctime_set_nsec(inode,
> +			     btrfs_timespec_nsec(leaf, &inode_item->ctime));
>  
>  	BTRFS_I(inode)->i_otime.tv_sec =
>  		btrfs_timespec_sec(leaf, &inode_item->otime);
> @@ -4073,9 +4075,9 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
>  				      inode->i_mtime.tv_nsec);
>  
>  	btrfs_set_token_timespec_sec(&token, &item->ctime,
> -				     inode->i_ctime.tv_sec);
> +				     inode_ctime_peek(inode).tv_sec);
>  	btrfs_set_token_timespec_nsec(&token, &item->ctime,
> -				      inode->i_ctime.tv_nsec);
> +				      inode_ctime_peek(inode).tv_nsec);
>  
>  	btrfs_set_token_timespec_sec(&token, &item->otime,
>  				     BTRFS_I(inode)->i_otime.tv_sec);
> @@ -4273,9 +4275,8 @@ static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans,
>  	btrfs_i_size_write(dir, dir->vfs_inode.i_size - name->len * 2);
>  	inode_inc_iversion(&inode->vfs_inode);
>  	inode_inc_iversion(&dir->vfs_inode);
> -	inode->vfs_inode.i_ctime = current_time(&inode->vfs_inode);
> -	dir->vfs_inode.i_mtime = inode->vfs_inode.i_ctime;
> -	dir->vfs_inode.i_ctime = inode->vfs_inode.i_ctime;
> +	inode_ctime_set_current(&inode->vfs_inode);
> +	dir->vfs_inode.i_mtime = inode_ctime_set_current(&dir->vfs_inode);
>  	ret = btrfs_update_inode(trans, root, dir);
>  out:
>  	return ret;
> @@ -4448,8 +4449,7 @@ static int btrfs_unlink_subvol(struct btrfs_trans_handle *trans,
>  
>  	btrfs_i_size_write(dir, dir->vfs_inode.i_size - fname.disk_name.len * 2);
>  	inode_inc_iversion(&dir->vfs_inode);
> -	dir->vfs_inode.i_mtime = current_time(&dir->vfs_inode);
> -	dir->vfs_inode.i_ctime = dir->vfs_inode.i_mtime;
> +	dir->vfs_inode.i_mtime = inode_ctime_set_current(&dir->vfs_inode);
>  	ret = btrfs_update_inode_fallback(trans, root, dir);
>  	if (ret)
>  		btrfs_abort_transaction(trans, ret);
> @@ -5090,10 +5090,8 @@ static int btrfs_setsize(struct inode *inode, struct iattr *attr)
>  	 */
>  	if (newsize != oldsize) {
>  		inode_inc_iversion(inode);
> -		if (!(mask & (ATTR_CTIME | ATTR_MTIME))) {
> -			inode->i_mtime = current_time(inode);
> -			inode->i_ctime = inode->i_mtime;
> -		}
> +		if (!(mask & (ATTR_CTIME | ATTR_MTIME)))
> +			inode->i_mtime = inode_ctime_set_current(inode);
>  	}
>  
>  	if (newsize > oldsize) {
> @@ -5736,9 +5734,7 @@ static struct inode *new_simple_dir(struct super_block *s,
>  	inode->i_opflags &= ~IOP_XATTR;
>  	inode->i_fop = &simple_dir_operations;
>  	inode->i_mode = S_IFDIR | S_IRUGO | S_IWUSR | S_IXUGO;
> -	inode->i_mtime = current_time(inode);
> -	inode->i_atime = inode->i_mtime;
> -	inode->i_ctime = inode->i_mtime;
> +	inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);

Please don't use chained assignments in btrfs code.

	inode->i_mtime = inode_ctime_set_current(inode);
	inode->i_atime = inode->i_mtime;

>  	BTRFS_I(inode)->i_otime = inode->i_mtime;
>  
>  	return inode;
> @@ -6075,7 +6071,7 @@ static int btrfs_update_time(struct inode *inode, struct timespec64 *now,
>  	if (flags & S_VERSION)
>  		dirty |= inode_maybe_inc_iversion(inode, dirty);
>  	if (flags & S_CTIME)
> -		inode->i_ctime = *now;
> +		inode_ctime_set(inode, *now);
>  	if (flags & S_MTIME)
>  		inode->i_mtime = *now;
>  	if (flags & S_ATIME)
> @@ -6378,9 +6374,7 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
>  		goto discard;
>  	}
>  
> -	inode->i_mtime = current_time(inode);
> -	inode->i_atime = inode->i_mtime;
> -	inode->i_ctime = inode->i_mtime;
> +	inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);

	inode->i_mtime = inode_ctime_set_current(inode);
	inode->i_atime = inode->i_mtime;

>  	BTRFS_I(inode)->i_otime = inode->i_mtime;
>  
>  	/*
> @@ -6545,12 +6539,9 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
>  	 * log replay procedure is responsible for setting them to their correct
>  	 * values (the ones it had when the fsync was done).
>  	 */
> -	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags)) {
> -		struct timespec64 now = current_time(&parent_inode->vfs_inode);
> +	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags))
> +		parent_inode->vfs_inode.i_mtime = inode_ctime_set_current(&parent_inode->vfs_inode);
>  
> -		parent_inode->vfs_inode.i_mtime = now;
> -		parent_inode->vfs_inode.i_ctime = now;
> -	}
>  	ret = btrfs_update_inode(trans, root, parent_inode);
>  	if (ret)
>  		btrfs_abort_transaction(trans, ret);
> @@ -6690,7 +6681,7 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
>  	BTRFS_I(inode)->dir_index = 0ULL;
>  	inc_nlink(inode);
>  	inode_inc_iversion(inode);
> -	inode->i_ctime = current_time(inode);
> +	inode_ctime_set_current(inode);
>  	ihold(inode);
>  	set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags);
>  
> @@ -8778,7 +8769,6 @@ static int btrfs_rename_exchange(struct inode *old_dir,
>  	struct btrfs_root *dest = BTRFS_I(new_dir)->root;
>  	struct inode *new_inode = new_dentry->d_inode;
>  	struct inode *old_inode = old_dentry->d_inode;
> -	struct timespec64 ctime = current_time(old_inode);
>  	struct btrfs_rename_ctx old_rename_ctx;
>  	struct btrfs_rename_ctx new_rename_ctx;
>  	u64 old_ino = btrfs_ino(BTRFS_I(old_inode));
> @@ -8909,12 +8899,10 @@ static int btrfs_rename_exchange(struct inode *old_dir,
>  	inode_inc_iversion(new_dir);
>  	inode_inc_iversion(old_inode);
>  	inode_inc_iversion(new_inode);
> -	old_dir->i_mtime = ctime;
> -	old_dir->i_ctime = ctime;
> -	new_dir->i_mtime = ctime;
> -	new_dir->i_ctime = ctime;
> -	old_inode->i_ctime = ctime;
> -	new_inode->i_ctime = ctime;
> +	old_dir->i_mtime = inode_ctime_set_current(old_dir);
> +	new_dir->i_mtime = inode_ctime_set_current(new_dir);
> +	inode_ctime_set_current(old_inode);
> +	inode_ctime_set_current(new_inode);
>  
>  	if (old_dentry->d_parent != new_dentry->d_parent) {
>  		btrfs_record_unlink_dir(trans, BTRFS_I(old_dir),
> @@ -9178,11 +9166,9 @@ static int btrfs_rename(struct mnt_idmap *idmap,
>  	inode_inc_iversion(old_dir);
>  	inode_inc_iversion(new_dir);
>  	inode_inc_iversion(old_inode);
> -	old_dir->i_mtime = current_time(old_dir);
> -	old_dir->i_ctime = old_dir->i_mtime;
> -	new_dir->i_mtime = old_dir->i_mtime;
> -	new_dir->i_ctime = old_dir->i_mtime;
> -	old_inode->i_ctime = old_dir->i_mtime;
> +	old_dir->i_mtime = inode_ctime_set_current(old_dir);
> +	new_dir->i_mtime = inode_ctime_set_current(new_dir);
> +	inode_ctime_set_current(old_inode);
>  
>  	if (old_dentry->d_parent != new_dentry->d_parent)
>  		btrfs_record_unlink_dir(trans, BTRFS_I(old_dir),
> @@ -9204,7 +9190,7 @@ static int btrfs_rename(struct mnt_idmap *idmap,
>  
>  	if (new_inode) {
>  		inode_inc_iversion(new_inode);
> -		new_inode->i_ctime = current_time(new_inode);
> +		inode_ctime_set_current(new_inode);
>  		if (unlikely(btrfs_ino(BTRFS_I(new_inode)) ==
>  			     BTRFS_EMPTY_SUBVOL_DIR_OBJECTID)) {
>  			ret = btrfs_unlink_subvol(trans, BTRFS_I(new_dir), new_dentry);
> @@ -9744,7 +9730,7 @@ static int __btrfs_prealloc_file_range(struct inode *inode, int mode,
>  		*alloc_hint = ins.objectid + ins.offset;
>  
>  		inode_inc_iversion(inode);
> -		inode->i_ctime = current_time(inode);
> +		inode_ctime_set_current(inode);
>  		BTRFS_I(inode)->flags |= BTRFS_INODE_PREALLOC;
>  		if (!(mode & FALLOC_FL_KEEP_SIZE) &&
>  		    (actual_len > inode->i_size) &&
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a895d105464b..3d50bd67ec85 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -384,7 +384,7 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
>  	binode->flags = binode_flags;
>  	btrfs_sync_inode_flags_to_i_flags(inode);
>  	inode_inc_iversion(inode);
> -	inode->i_ctime = current_time(inode);
> +	inode_ctime_set_current(inode);
>  	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
>  
>   out_end_trans:
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index 0474bbe39da7..3cf4716dea46 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -29,10 +29,9 @@ static int clone_finish_inode_update(struct btrfs_trans_handle *trans,
>  	int ret;
>  
>  	inode_inc_iversion(inode);
> -	if (!no_time_update) {
> -		inode->i_mtime = current_time(inode);
> -		inode->i_ctime = inode->i_mtime;
> -	}
> +	if (!no_time_update)
> +		inode->i_mtime = inode_ctime_set_current(inode);
> +
>  	/*
>  	 * We round up to the block size at eof when determining which
>  	 * extents to clone above, but shouldn't round up the file size.
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index e7cfc992e02a..138f919646e2 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1831,8 +1831,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
>  
>  	btrfs_i_size_write(BTRFS_I(parent_inode), parent_inode->i_size +
>  						  fname.disk_name.len * 2);
> -	parent_inode->i_mtime = current_time(parent_inode);
> -	parent_inode->i_ctime = parent_inode->i_mtime;
> +	parent_inode->i_mtime = inode_ctime_set_current(parent_inode);
>  	ret = btrfs_update_inode_fallback(trans, parent_root, BTRFS_I(parent_inode));
>  	if (ret) {
>  		btrfs_abort_transaction(trans, ret);
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 365a1cc0a3c3..c862d1450365 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -4148,9 +4148,9 @@ static void fill_inode_item(struct btrfs_trans_handle *trans,
>  				      inode->i_mtime.tv_nsec);
>  
>  	btrfs_set_token_timespec_sec(&token, &item->ctime,
> -				     inode->i_ctime.tv_sec);
> +				     inode_ctime_peek(inode).tv_sec);
>  	btrfs_set_token_timespec_nsec(&token, &item->ctime,
> -				      inode->i_ctime.tv_nsec);
> +				      inode_ctime_peek(inode).tv_nsec);
>  
>  	/*
>  	 * We do not need to set the nbytes field, in fact during a fast fsync
> diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> index fc4b20c2688a..a2d331436963 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -264,7 +264,7 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
>  		goto out;
>  
>  	inode_inc_iversion(inode);
> -	inode->i_ctime = current_time(inode);
> +	inode_ctime_set_current(inode);
>  	ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
>  	if (ret)
>  		btrfs_abort_transaction(trans, ret);
> @@ -407,7 +407,7 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
>  	ret = btrfs_set_prop(trans, inode, name, value, size, flags);
>  	if (!ret) {
>  		inode_inc_iversion(inode);
> -		inode->i_ctime = current_time(inode);
> +		inode_ctime_set_current(inode);
>  		ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
>  		if (ret)
>  			btrfs_abort_transaction(trans, ret);
> -- 
> 2.41.0
> 

Re: [PATCH 00/79] fs: new accessors for inode->i_ctime

[Christian Brauner]

On Wed, Jun 21, 2023 at 03:52:27PM -0400, Jeff Layton wrote:
> On Wed, 2023-06-21 at 15:21 -0400, Steven Rostedt wrote:
> > On Wed, 21 Jun 2023 10:45:05 -0400
> > Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > > Most of this conversion was done via coccinelle, with a few of the more
> > > non-standard accesses done by hand. There should be no behavioral
> > > changes with this set. That will come later, as we convert individual
> > > filesystems to use multigrain timestamps.
> > 
> > BTW, Linus has suggested to me that whenever a conccinelle script is used,
> > it should be included in the change log.
> > 
> 
> Ok, here's what I have. I note again that my usage of coccinelle is
> pretty primitive, so I ended up doing a fair bit of by-hand fixing after
> applying these.
> 
> Given the way that this change is broken up into 77 patches by
> subsystem, to which changelogs should I add it? I could add it to the
> "infrastructure" patch, but that's the one where I _didn't_ use it. 
> 
> Maybe to patch #79 (the one that renames i_ctime)?

That works. I can also put this into a merge commit or pr message.

Re: [PATCH 00/79] fs: new accessors for inode->i_ctime

[Luis Chamberlain]

On Wed, Jun 21, 2023 at 03:21:41PM -0400, Steven Rostedt wrote:
> On Wed, 21 Jun 2023 10:45:05 -0400
> Jeff Layton <jlayton@kernel.org> wrote:
> 
> > Most of this conversion was done via coccinelle, with a few of the more
> > non-standard accesses done by hand. There should be no behavioral
> > changes with this set. That will come later, as we convert individual
> > filesystems to use multigrain timestamps.
> 
> BTW, Linus has suggested to me that whenever a conccinelle script is used,
> it should be included in the change log.

Sometimes people like the coccinelle included in the commit, sometimes
people don't [0], it really ends up being up to a subjective maintainer
preference. A compromise could be to use git notes as these are
optional, however if we want to go down that path we should try to make
a general consensus on it so we can send a consistent message.

[0] https://lore.kernel.org/all/20230512073100.GC32559@twin.jikos.cz/

  Luis

Re: [PATCH 01/79] fs: add ctime accessors infrastructure

[Luis Chamberlain]

On Wed, Jun 21, 2023 at 10:45:06AM -0400, Jeff Layton wrote:
> struct timespec64 has unused bits in the tv_nsec field that can be used
> for other purposes. In future patches, we're going to change how the
> inode->i_ctime is accessed in certain inodes in order to make use of
> them. In order to do that safely though, we'll need to eradicate raw
> accesses of the inode->i_ctime field from the kernel.
> 
> Add new accessor functions for the ctime that we can use to replace them.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

Re: [PATCH 01/79] fs: add ctime accessors infrastructure

[Randy Dunlap]

Hi Jeff,

On arch/um/, (subarch i386 or x86_64), hostfs build fails with:

../fs/hostfs/hostfs_kern.c:520:36: error: incompatible type for arg
ument 2 of 'inode_set_ctime_to_ts'
../include/linux/fs.h:1499:73: note: expected 'struct timespec64' b
ut argument is of type 'const struct hostfs_timespec *'


-- 
~Randy