* [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime
@ 2023-06-21 14:45 Jeff Layton
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 14:45 UTC (permalink / raw)
To: cluster-devel.redhat.com
I've been working on a patchset to change how the inode->i_ctime is
accessed in order to give us conditional, high-res timestamps for the
ctime and mtime. struct timespec64 has unused bits in it that we can use
to implement this. In order to do that however, we need to wrap all
accesses of inode->i_ctime to ensure that bits used as flags are
appropriately handled.
This patchset first adds some new inode_ctime_* accessor functions. It
then converts all in-tree accesses of inode->i_ctime to use those new
functions and then renames the i_ctime field to __i_ctime to help ensure
that use of the accessors.
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.
Some of these patches depend on the set I sent recently to add missing
ctime updates in various subsystems:
https://lore.kernel.org/linux-fsdevel/20230612104524.17058-1-jlayton at kernel.org/T/#m25399f903cc9526e46b2e0f5a35713c80b52fde9
Since this patchset is so large, I'm only going to send individual
conversion patches to the appropriate maintainers. Please send
Acked-by's or Reviewed-by's if you can. The intent is to merge these as
a set (probably in v6.6). Let me know if that causes conflicts though,
and we can work it out.
This is based on top of linux-next as of yesterday.
Jeff Layton (79):
fs: add ctime accessors infrastructure
spufs: switch to new ctime accessors
s390: switch to new ctime accessors
binderfs: switch to new ctime accessors
qib_fs: switch to new ctime accessors
ibm: switch to new ctime accessors
usb: switch to new ctime accessors
9p: switch to new ctime accessors
adfs: switch to new ctime accessors
affs: switch to new ctime accessors
afs: switch to new ctime accessors
fs: switch to new ctime accessors
autofs: switch to new ctime accessors
befs: switch to new ctime accessors
bfs: switch to new ctime accessors
btrfs: switch to new ctime accessors
ceph: switch to new ctime accessors
coda: switch to new ctime accessors
configfs: switch to new ctime accessors
cramfs: switch to new ctime accessors
debugfs: switch to new ctime accessors
devpts: switch to new ctime accessors
ecryptfs: switch to new ctime accessors
efivarfs: switch to new ctime accessors
efs: switch to new ctime accessors
erofs: switch to new ctime accessors
exfat: switch to new ctime accessors
ext2: switch to new ctime accessors
ext4: switch to new ctime accessors
f2fs: switch to new ctime accessors
fat: switch to new ctime accessors
freevxfs: switch to new ctime accessors
fuse: switch to new ctime accessors
gfs2: switch to new ctime accessors
hfs: switch to new ctime accessors
hfsplus: switch to new ctime accessors
hostfs: switch to new ctime accessors
hpfs: switch to new ctime accessors
hugetlbfs: switch to new ctime accessors
isofs: switch to new ctime accessors
jffs2: switch to new ctime accessors
jfs: switch to new ctime accessors
kernfs: switch to new ctime accessors
minix: switch to new ctime accessors
nfs: switch to new ctime accessors
nfsd: switch to new ctime accessors
nilfs2: switch to new ctime accessors
ntfs: switch to new ctime accessors
ntfs3: switch to new ctime accessors
ocfs2: switch to new ctime accessors
omfs: switch to new ctime accessors
openpromfs: switch to new ctime accessors
orangefs: switch to new ctime accessors
overlayfs: switch to new ctime accessors
proc: switch to new ctime accessors
pstore: switch to new ctime accessors
qnx4: switch to new ctime accessors
qnx6: switch to new ctime accessors
ramfs: switch to new ctime accessors
reiserfs: switch to new ctime accessors
romfs: switch to new ctime accessors
smb: switch to new ctime accessors
squashfs: switch to new ctime accessors
sysv: switch to new ctime accessors
tracefs: switch to new ctime accessors
ubifs: switch to new ctime accessors
udf: switch to new ctime accessors
ufs: switch to new ctime accessors
vboxsf: switch to new ctime accessors
xfs: switch to new ctime accessors
zonefs: switch to new ctime accessors
mqueue: switch to new ctime accessors
bpf: switch to new ctime accessors
shmem: switch to new ctime accessors
rpc_pipefs: switch to new ctime accessors
apparmor: switch to new ctime accessors
security: switch to new ctime accessors
selinux: switch to new ctime accessors
fs: rename i_ctime field to __i_ctime
arch/powerpc/platforms/cell/spufs/inode.c | 2 +-
arch/s390/hypfs/inode.c | 4 +-
drivers/android/binderfs.c | 8 +--
drivers/infiniband/hw/qib/qib_fs.c | 4 +-
drivers/misc/ibmasm/ibmasmfs.c | 2 +-
drivers/misc/ibmvmc.c | 2 +-
drivers/usb/core/devio.c | 16 +++---
drivers/usb/gadget/function/f_fs.c | 6 +--
drivers/usb/gadget/legacy/inode.c | 3 +-
fs/9p/vfs_inode.c | 6 ++-
fs/9p/vfs_inode_dotl.c | 11 ++--
fs/adfs/inode.c | 4 +-
fs/affs/amigaffs.c | 6 +--
fs/affs/inode.c | 17 +++---
fs/afs/dynroot.c | 2 +-
fs/afs/inode.c | 6 +--
fs/attr.c | 2 +-
fs/autofs/inode.c | 2 +-
fs/autofs/root.c | 6 +--
fs/bad_inode.c | 3 +-
fs/befs/linuxvfs.c | 2 +-
fs/bfs/dir.c | 16 +++---
fs/bfs/inode.c | 6 +--
fs/binfmt_misc.c | 3 +-
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 +-
fs/ceph/acl.c | 2 +-
fs/ceph/caps.c | 2 +-
fs/ceph/inode.c | 17 +++---
fs/ceph/snap.c | 2 +-
fs/ceph/xattr.c | 2 +-
fs/coda/coda_linux.c | 2 +-
fs/coda/dir.c | 2 +-
fs/coda/file.c | 2 +-
fs/coda/inode.c | 2 +-
fs/configfs/inode.c | 6 +--
fs/cramfs/inode.c | 2 +-
fs/debugfs/inode.c | 2 +-
fs/devpts/inode.c | 6 +--
fs/ecryptfs/inode.c | 2 +-
fs/efivarfs/file.c | 2 +-
fs/efivarfs/inode.c | 2 +-
fs/efs/inode.c | 5 +-
fs/erofs/inode.c | 16 +++---
fs/exfat/file.c | 4 +-
fs/exfat/inode.c | 6 +--
fs/exfat/namei.c | 29 +++++-----
fs/exfat/super.c | 4 +-
fs/ext2/acl.c | 2 +-
fs/ext2/dir.c | 6 +--
fs/ext2/ialloc.c | 2 +-
fs/ext2/inode.c | 11 ++--
fs/ext2/ioctl.c | 4 +-
fs/ext2/namei.c | 8 +--
fs/ext2/super.c | 2 +-
fs/ext2/xattr.c | 2 +-
fs/ext4/acl.c | 2 +-
fs/ext4/ext4.h | 20 +++++++
fs/ext4/extents.c | 12 ++---
fs/ext4/ialloc.c | 2 +-
fs/ext4/inline.c | 4 +-
fs/ext4/inode.c | 16 +++---
fs/ext4/ioctl.c | 9 ++--
fs/ext4/namei.c | 26 +++++----
fs/ext4/super.c | 2 +-
fs/ext4/xattr.c | 6 +--
fs/f2fs/dir.c | 8 +--
fs/f2fs/f2fs.h | 5 +-
fs/f2fs/file.c | 16 +++---
fs/f2fs/inline.c | 2 +-
fs/f2fs/inode.c | 10 ++--
fs/f2fs/namei.c | 12 ++---
fs/f2fs/recovery.c | 4 +-
fs/f2fs/super.c | 2 +-
fs/f2fs/xattr.c | 2 +-
fs/fat/inode.c | 8 +--
fs/fat/misc.c | 7 ++-
fs/freevxfs/vxfs_inode.c | 4 +-
fs/fuse/control.c | 2 +-
fs/fuse/dir.c | 8 +--
fs/fuse/inode.c | 18 ++++---
fs/gfs2/acl.c | 2 +-
fs/gfs2/bmap.c | 11 ++--
fs/gfs2/dir.c | 15 +++---
fs/gfs2/file.c | 2 +-
fs/gfs2/glops.c | 4 +-
fs/gfs2/inode.c | 8 +--
fs/gfs2/super.c | 4 +-
fs/gfs2/xattr.c | 8 +--
fs/hfs/catalog.c | 8 +--
fs/hfs/dir.c | 2 +-
fs/hfs/inode.c | 13 +++--
fs/hfs/sysdep.c | 2 +-
fs/hfsplus/catalog.c | 8 +--
fs/hfsplus/dir.c | 6 +--
fs/hfsplus/inode.c | 14 ++---
fs/hostfs/hostfs_kern.c | 4 +-
fs/hpfs/dir.c | 8 +--
fs/hpfs/inode.c | 6 +--
fs/hpfs/namei.c | 26 +++++----
fs/hpfs/super.c | 5 +-
fs/hugetlbfs/inode.c | 12 ++---
fs/inode.c | 26 +++++++--
fs/isofs/inode.c | 4 +-
fs/isofs/rock.c | 16 +++---
fs/jffs2/dir.c | 19 +++----
fs/jffs2/file.c | 3 +-
fs/jffs2/fs.c | 10 ++--
fs/jffs2/os-linux.h | 2 +-
fs/jfs/acl.c | 2 +-
fs/jfs/inode.c | 2 +-
fs/jfs/ioctl.c | 2 +-
fs/jfs/jfs_imap.c | 8 +--
fs/jfs/jfs_inode.c | 4 +-
fs/jfs/namei.c | 25 ++++-----
fs/jfs/super.c | 2 +-
fs/jfs/xattr.c | 2 +-
fs/kernfs/inode.c | 4 +-
fs/libfs.c | 32 +++++------
fs/minix/bitmap.c | 2 +-
fs/minix/dir.c | 6 +--
fs/minix/inode.c | 11 ++--
fs/minix/itree_common.c | 4 +-
fs/minix/namei.c | 6 +--
fs/nfs/callback_proc.c | 2 +-
fs/nfs/fscache.h | 4 +-
fs/nfs/inode.c | 21 ++++----
fs/nfsd/nfsctl.c | 2 +-
fs/nfsd/vfs.c | 2 +-
fs/nilfs2/dir.c | 6 +--
fs/nilfs2/inode.c | 12 ++---
fs/nilfs2/ioctl.c | 2 +-
fs/nilfs2/namei.c | 8 +--
fs/nsfs.c | 2 +-
fs/ntfs/inode.c | 15 +++---
fs/ntfs/mft.c | 3 +-
fs/ntfs3/file.c | 6 +--
fs/ntfs3/frecord.c | 4 +-
fs/ntfs3/inode.c | 14 ++---
fs/ntfs3/namei.c | 10 ++--
fs/ntfs3/xattr.c | 4 +-
fs/ocfs2/acl.c | 6 +--
fs/ocfs2/alloc.c | 6 +--
fs/ocfs2/aops.c | 2 +-
fs/ocfs2/dir.c | 8 +--
fs/ocfs2/dlmfs/dlmfs.c | 4 +-
fs/ocfs2/dlmglue.c | 10 ++--
fs/ocfs2/file.c | 16 +++---
fs/ocfs2/inode.c | 14 ++---
fs/ocfs2/move_extents.c | 6 +--
fs/ocfs2/namei.c | 22 ++++----
fs/ocfs2/refcounttree.c | 14 ++---
fs/ocfs2/xattr.c | 6 +--
fs/omfs/dir.c | 4 +-
fs/omfs/inode.c | 10 ++--
fs/openpromfs/inode.c | 4 +-
fs/orangefs/namei.c | 2 +-
fs/orangefs/orangefs-utils.c | 6 +--
fs/overlayfs/file.c | 7 ++-
fs/overlayfs/util.c | 2 +-
fs/pipe.c | 2 +-
fs/posix_acl.c | 2 +-
fs/proc/base.c | 2 +-
fs/proc/inode.c | 2 +-
fs/proc/proc_sysctl.c | 2 +-
fs/proc/self.c | 2 +-
fs/proc/thread_self.c | 2 +-
fs/pstore/inode.c | 4 +-
fs/qnx4/inode.c | 4 +-
fs/qnx6/inode.c | 4 +-
fs/ramfs/inode.c | 6 +--
fs/reiserfs/inode.c | 14 ++---
fs/reiserfs/ioctl.c | 4 +-
fs/reiserfs/namei.c | 21 ++++----
fs/reiserfs/stree.c | 4 +-
fs/reiserfs/super.c | 2 +-
fs/reiserfs/xattr.c | 5 +-
fs/reiserfs/xattr_acl.c | 2 +-
fs/romfs/super.c | 4 +-
fs/smb/client/file.c | 4 +-
fs/smb/client/fscache.h | 5 +-
fs/smb/client/inode.c | 15 +++---
fs/smb/client/smb2ops.c | 2 +-
fs/smb/server/smb2pdu.c | 8 +--
fs/squashfs/inode.c | 2 +-
fs/stack.c | 2 +-
fs/stat.c | 2 +-
fs/sysv/dir.c | 6 +--
fs/sysv/ialloc.c | 2 +-
fs/sysv/inode.c | 6 +--
fs/sysv/itree.c | 4 +-
fs/sysv/namei.c | 6 +--
fs/tracefs/inode.c | 2 +-
fs/ubifs/debug.c | 4 +-
fs/ubifs/dir.c | 39 +++++++-------
fs/ubifs/file.c | 16 +++---
fs/ubifs/ioctl.c | 2 +-
fs/ubifs/journal.c | 4 +-
fs/ubifs/super.c | 4 +-
fs/ubifs/xattr.c | 6 +--
fs/udf/ialloc.c | 2 +-
fs/udf/inode.c | 17 +++---
fs/udf/namei.c | 24 ++++-----
fs/ufs/dir.c | 6 +--
fs/ufs/ialloc.c | 2 +-
fs/ufs/inode.c | 23 ++++----
fs/ufs/namei.c | 8 +--
fs/vboxsf/utils.c | 4 +-
fs/xfs/libxfs/xfs_inode_buf.c | 4 +-
fs/xfs/libxfs/xfs_trans_inode.c | 2 +-
fs/xfs/xfs_acl.c | 2 +-
fs/xfs/xfs_bmap_util.c | 6 ++-
fs/xfs/xfs_inode.c | 2 +-
fs/xfs/xfs_inode_item.c | 2 +-
fs/xfs/xfs_iops.c | 4 +-
fs/xfs/xfs_itable.c | 4 +-
fs/zonefs/super.c | 8 +--
include/linux/fs.h | 55 ++++++++++++++++++-
include/linux/fs_stack.h | 2 +-
ipc/mqueue.c | 20 ++++---
kernel/bpf/inode.c | 4 +-
mm/shmem.c | 28 +++++-----
net/sunrpc/rpc_pipe.c | 2 +-
security/apparmor/apparmorfs.c | 6 +--
security/apparmor/policy_unpack.c | 4 +-
security/inode.c | 2 +-
security/selinux/selinuxfs.c | 2 +-
233 files changed, 914 insertions(+), 808 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Jeff Layton
@ 2023-06-21 14:45 ` Jeff Layton
2023-06-21 16:34 ` Jan Kara
` (4 more replies)
[not found] ` <20230621144735.55953-1-jlayton@kernel.org>
` (2 subsequent siblings)
3 siblings, 5 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 14:45 UTC (permalink / raw)
To: cluster-devel.redhat.com
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>
---
fs/inode.c | 16 ++++++++++++++
include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index d37fad91c8da..c005e7328fbb 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
}
EXPORT_SYMBOL(current_time);
+/**
+ * inode_ctime_set_current - set the ctime to current_time
+ * @inode: inode
+ *
+ * Set the inode->i_ctime to the current value for the inode. Returns
+ * the current value that was assigned to i_ctime.
+ */
+struct timespec64 inode_ctime_set_current(struct inode *inode)
+{
+ struct timespec64 now = current_time(inode);
+
+ inode_set_ctime(inode, now);
+ return now;
+}
+EXPORT_SYMBOL(inode_ctime_set_current);
+
/**
* in_group_or_capable - check whether caller is CAP_FSETID privileged
* @idmap: idmap of the mount @inode was found from
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6867512907d6..9afb30606373 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
kgid_has_mapping(fs_userns, kgid);
}
-extern struct timespec64 current_time(struct inode *inode);
+struct timespec64 current_time(struct inode *inode);
+struct timespec64 inode_ctime_set_current(struct inode *inode);
+
+/**
+ * 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)
+{
+ return inode->i_ctime;
+}
+
+/**
+ * inode_ctime_set - set the ctime in the inode to the given value
+ * @inode: inode in which to set the ctime
+ * @ts: timespec value to set the ctime
+ *
+ * Set the ctime in @inode to @ts.
+ */
+static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
+{
+ inode->i_ctime = ts;
+ return ts;
+}
+
+/**
+ * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @sec: value to set the tv_sec field
+ *
+ * Set the sec field in the ctime. Returns @sec.
+ */
+static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
+{
+ inode->i_ctime.tv_sec = sec;
+ return sec;
+}
+
+/**
+ * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
+ * @inode: inode in which to set the ctime
+ * @nsec: value to set the tv_nsec field
+ *
+ * Set the nsec field in the ctime. Returns @nsec.
+ */
+static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
+{
+ inode->i_ctime.tv_nsec = nsec;
+ return nsec;
+}
/*
* Snapshotting support.
--
2.41.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 34/79] gfs2: switch to new ctime accessors
[not found] ` <20230621144735.55953-1-jlayton@kernel.org>
@ 2023-06-21 14:45 ` Jeff Layton
0 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 14:45 UTC (permalink / raw)
To: cluster-devel.redhat.com
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/gfs2/acl.c | 2 +-
fs/gfs2/bmap.c | 11 +++++------
fs/gfs2/dir.c | 15 ++++++++-------
fs/gfs2/file.c | 2 +-
fs/gfs2/glops.c | 4 ++--
fs/gfs2/inode.c | 8 ++++----
fs/gfs2/super.c | 4 ++--
fs/gfs2/xattr.c | 8 ++++----
8 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
index a392aa0f041d..b267dae0dc63 100644
--- a/fs/gfs2/acl.c
+++ b/fs/gfs2/acl.c
@@ -142,7 +142,7 @@ int gfs2_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
ret = __gfs2_set_acl(inode, acl, type);
if (!ret && mode != inode->i_mode) {
- inode->i_ctime = current_time(inode);
+ inode_ctime_set_current(inode);
inode->i_mode = mode;
mark_inode_dirty(inode);
}
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 8d611fbcf0bd..743b09a0b196 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -1386,7 +1386,7 @@ static int trunc_start(struct inode *inode, u64 newsize)
ip->i_diskflags |= GFS2_DIF_TRUNC_IN_PROG;
i_size_write(inode, newsize);
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode->i_mtime = inode_ctime_set_current(inode);
gfs2_dinode_out(ip, dibh->b_data);
if (journaled)
@@ -1583,8 +1583,7 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip, struct gfs2_holder *rd_gh,
/* Every transaction boundary, we rewrite the dinode
to keep its di_blocks current in case of failure. */
- ip->i_inode.i_mtime = ip->i_inode.i_ctime =
- current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
gfs2_trans_add_meta(ip->i_gl, dibh);
gfs2_dinode_out(ip, dibh->b_data);
brelse(dibh);
@@ -1950,7 +1949,7 @@ static int punch_hole(struct gfs2_inode *ip, u64 offset, u64 length)
gfs2_statfs_change(sdp, 0, +btotal, 0);
gfs2_quota_change(ip, -(s64)btotal, ip->i_inode.i_uid,
ip->i_inode.i_gid);
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
gfs2_trans_add_meta(ip->i_gl, dibh);
gfs2_dinode_out(ip, dibh->b_data);
up_write(&ip->i_rw_mutex);
@@ -1993,7 +1992,7 @@ static int trunc_end(struct gfs2_inode *ip)
gfs2_buffer_clear_tail(dibh, sizeof(struct gfs2_dinode));
gfs2_ordered_del_inode(ip);
}
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
ip->i_diskflags &= ~GFS2_DIF_TRUNC_IN_PROG;
gfs2_trans_add_meta(ip->i_gl, dibh);
@@ -2094,7 +2093,7 @@ static int do_grow(struct inode *inode, u64 size)
goto do_end_trans;
truncate_setsize(inode, size);
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
gfs2_trans_add_meta(ip->i_gl, dibh);
gfs2_dinode_out(ip, dibh->b_data);
brelse(dibh);
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 54a6d17b8c25..c07cb9883ea1 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -130,7 +130,7 @@ static int gfs2_dir_write_stuffed(struct gfs2_inode *ip, const char *buf,
memcpy(dibh->b_data + offset + sizeof(struct gfs2_dinode), buf, size);
if (ip->i_inode.i_size < offset + size)
i_size_write(&ip->i_inode, offset + size);
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
gfs2_dinode_out(ip, dibh->b_data);
brelse(dibh);
@@ -227,7 +227,7 @@ static int gfs2_dir_write_data(struct gfs2_inode *ip, const char *buf,
if (ip->i_inode.i_size < offset + copied)
i_size_write(&ip->i_inode, offset + copied);
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ ip->i_inode.i_mtime = inode_ctime_set_current(&ip->i_inode);
gfs2_trans_add_meta(ip->i_gl, dibh);
gfs2_dinode_out(ip, dibh->b_data);
@@ -1814,7 +1814,7 @@ int gfs2_dir_add(struct inode *inode, const struct qstr *name,
gfs2_inum_out(nip, dent);
dent->de_type = cpu_to_be16(IF2DT(nip->i_inode.i_mode));
dent->de_rahead = cpu_to_be16(gfs2_inode_ra_len(nip));
- tv = current_time(&ip->i_inode);
+ tv = inode_ctime_set_current(&ip->i_inode);
if (ip->i_diskflags & GFS2_DIF_EXHASH) {
leaf = (struct gfs2_leaf *)bh->b_data;
be16_add_cpu(&leaf->lf_entries, 1);
@@ -1825,7 +1825,7 @@ int gfs2_dir_add(struct inode *inode, const struct qstr *name,
da->bh = NULL;
brelse(bh);
ip->i_entries++;
- ip->i_inode.i_mtime = ip->i_inode.i_ctime = tv;
+ ip->i_inode.i_mtime = tv;
if (S_ISDIR(nip->i_inode.i_mode))
inc_nlink(&ip->i_inode);
mark_inode_dirty(inode);
@@ -1876,7 +1876,7 @@ int gfs2_dir_del(struct gfs2_inode *dip, const struct dentry *dentry)
const struct qstr *name = &dentry->d_name;
struct gfs2_dirent *dent, *prev = NULL;
struct buffer_head *bh;
- struct timespec64 tv = current_time(&dip->i_inode);
+ struct timespec64 tv;
/* Returns _either_ the entry (if its first in block) or the
previous entry otherwise */
@@ -1895,6 +1895,7 @@ int gfs2_dir_del(struct gfs2_inode *dip, const struct dentry *dentry)
dent = (struct gfs2_dirent *)((char *)dent + be16_to_cpu(prev->de_rec_len));
}
+ tv = inode_ctime_set_current(&dip->i_inode);
dirent_del(dip, bh, prev, dent);
if (dip->i_diskflags & GFS2_DIF_EXHASH) {
struct gfs2_leaf *leaf = (struct gfs2_leaf *)bh->b_data;
@@ -1910,7 +1911,7 @@ int gfs2_dir_del(struct gfs2_inode *dip, const struct dentry *dentry)
if (!dip->i_entries)
gfs2_consist_inode(dip);
dip->i_entries--;
- dip->i_inode.i_mtime = dip->i_inode.i_ctime = tv;
+ dip->i_inode.i_mtime = tv;
if (d_is_dir(dentry))
drop_nlink(&dip->i_inode);
mark_inode_dirty(&dip->i_inode);
@@ -1951,7 +1952,7 @@ int gfs2_dir_mvino(struct gfs2_inode *dip, const struct qstr *filename,
dent->de_type = cpu_to_be16(new_type);
brelse(bh);
- dip->i_inode.i_mtime = dip->i_inode.i_ctime = current_time(&dip->i_inode);
+ dip->i_inode.i_mtime = inode_ctime_set_current(&dip->i_inode);
mark_inode_dirty_sync(&dip->i_inode);
return 0;
}
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 1bf3c4453516..cb754c5f1d2d 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -260,7 +260,7 @@ static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask)
error = gfs2_meta_inode_buffer(ip, &bh);
if (error)
goto out_trans_end;
- inode->i_ctime = current_time(inode);
+ inode_ctime_set_current(inode);
gfs2_trans_add_meta(ip->i_gl, bh);
ip->i_diskflags = new_flags;
gfs2_dinode_out(ip, bh->b_data);
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 7c48c7afd6a4..2aba6f82194f 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -437,8 +437,8 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
inode->i_atime = atime;
inode->i_mtime.tv_sec = be64_to_cpu(str->di_mtime);
inode->i_mtime.tv_nsec = be32_to_cpu(str->di_mtime_nsec);
- inode->i_ctime.tv_sec = be64_to_cpu(str->di_ctime);
- inode->i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
+ inode_ctime_set_sec(inode, be64_to_cpu(str->di_ctime));
+ inode_ctime_set_nsec(inode, be32_to_cpu(str->di_ctime_nsec));
ip->i_goal = be64_to_cpu(str->di_goal_meta);
ip->i_generation = be64_to_cpu(str->di_generation);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 17c994a0c0d0..3a9c9b6ea456 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -690,7 +690,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
set_nlink(inode, S_ISDIR(mode) ? 2 : 1);
inode->i_rdev = dev;
inode->i_size = size;
- inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+ inode->i_atime = inode->i_mtime = inode_ctime_set_current(inode);
munge_mode_uid_gid(dip, inode);
check_and_update_goal(dip);
ip->i_goal = dip->i_goal;
@@ -1029,7 +1029,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
gfs2_trans_add_meta(ip->i_gl, dibh);
inc_nlink(&ip->i_inode);
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
ihold(inode);
d_instantiate(dentry, inode);
mark_inode_dirty(inode);
@@ -1114,7 +1114,7 @@ static int gfs2_unlink_inode(struct gfs2_inode *dip,
return error;
ip->i_entries = 0;
- inode->i_ctime = current_time(inode);
+ inode_ctime_set_current(inode);
if (S_ISDIR(inode->i_mode))
clear_nlink(inode);
else
@@ -1371,7 +1371,7 @@ static int update_moved_ino(struct gfs2_inode *ip, struct gfs2_inode *ndip,
if (dir_rename)
return gfs2_dir_mvino(ip, &gfs2_qdotdot, ndip, DT_DIR);
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
mark_inode_dirty_sync(&ip->i_inode);
return 0;
}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 9be72d5aafea..fc495df15e71 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -412,7 +412,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
str->di_blocks = cpu_to_be64(gfs2_get_inode_blocks(inode));
str->di_atime = cpu_to_be64(inode->i_atime.tv_sec);
str->di_mtime = cpu_to_be64(inode->i_mtime.tv_sec);
- str->di_ctime = cpu_to_be64(inode->i_ctime.tv_sec);
+ str->di_ctime = cpu_to_be64(inode_ctime_peek(inode).tv_sec);
str->di_goal_meta = cpu_to_be64(ip->i_goal);
str->di_goal_data = cpu_to_be64(ip->i_goal);
@@ -429,7 +429,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
str->di_eattr = cpu_to_be64(ip->i_eattr);
str->di_atime_nsec = cpu_to_be32(inode->i_atime.tv_nsec);
str->di_mtime_nsec = cpu_to_be32(inode->i_mtime.tv_nsec);
- str->di_ctime_nsec = cpu_to_be32(inode->i_ctime.tv_nsec);
+ str->di_ctime_nsec = cpu_to_be32(inode_ctime_peek(inode).tv_nsec);
}
/**
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 93b36d026bb4..8f807d18ec52 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -311,7 +311,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
ea->ea_num_ptrs = 0;
}
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
__mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(sdp);
@@ -763,7 +763,7 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
if (error)
goto out_end_trans;
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
__mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
out_end_trans:
@@ -888,7 +888,7 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
if (es->es_el)
ea_set_remove_stuffed(ip, es->es_el);
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
__mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(GFS2_SB(&ip->i_inode));
@@ -1106,7 +1106,7 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
ea->ea_type = GFS2_EATYPE_UNUSED;
}
- ip->i_inode.i_ctime = current_time(&ip->i_inode);
+ inode_ctime_set_current(&ip->i_inode);
__mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(GFS2_SB(&ip->i_inode));
--
2.41.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 79/79] fs: rename i_ctime field to __i_ctime
2023-06-21 14:45 [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Jeff Layton
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
[not found] ` <20230621144735.55953-1-jlayton@kernel.org>
@ 2023-06-21 14:49 ` Jeff Layton
2023-06-21 19:21 ` [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Steven Rostedt
3 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 14:49 UTC (permalink / raw)
To: cluster-devel.redhat.com
Now that everything in-tree is converted to use the accessor functions,
rename the i_ctime field in the inode to make its accesses more
self-documenting.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
include/linux/fs.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9afb30606373..2ca46c532b49 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -642,7 +642,7 @@ struct inode {
loff_t i_size;
struct timespec64 i_atime;
struct timespec64 i_mtime;
- struct timespec64 i_ctime;
+ struct timespec64 __i_ctime; /* use inode_ctime_* accessors! */
spinlock_t i_lock; /* i_blocks, i_bytes, maybe i_size */
unsigned short i_bytes;
u8 i_blkbits;
@@ -1485,7 +1485,7 @@ struct timespec64 inode_ctime_set_current(struct inode *inode);
*/
static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
{
- return inode->i_ctime;
+ return inode->__i_ctime;
}
/**
@@ -1497,7 +1497,7 @@ static inline struct timespec64 inode_ctime_peek(const struct inode *inode)
*/
static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
{
- inode->i_ctime = ts;
+ inode->__i_ctime = ts;
return ts;
}
@@ -1510,7 +1510,7 @@ static inline struct timespec64 inode_ctime_set(struct inode *inode, struct time
*/
static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
{
- inode->i_ctime.tv_sec = sec;
+ inode->__i_ctime.tv_sec = sec;
return sec;
}
@@ -1523,7 +1523,7 @@ static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
*/
static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
{
- inode->i_ctime.tv_nsec = nsec;
+ inode->__i_ctime.tv_nsec = nsec;
return nsec;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
@ 2023-06-21 16:34 ` Jan Kara
2023-06-21 17:29 ` Tom Talpey
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Jan Kara @ 2023-06-21 16:34 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Wed 21-06-23 10:45:06, 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>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/inode.c | 16 ++++++++++++++
> include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index d37fad91c8da..c005e7328fbb 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> }
> EXPORT_SYMBOL(current_time);
>
> +/**
> + * inode_ctime_set_current - set the ctime to current_time
> + * @inode: inode
> + *
> + * Set the inode->i_ctime to the current value for the inode. Returns
> + * the current value that was assigned to i_ctime.
> + */
> +struct timespec64 inode_ctime_set_current(struct inode *inode)
> +{
> + struct timespec64 now = current_time(inode);
> +
> + inode_set_ctime(inode, now);
> + return now;
> +}
> +EXPORT_SYMBOL(inode_ctime_set_current);
> +
> /**
> * in_group_or_capable - check whether caller is CAP_FSETID privileged
> * @idmap: idmap of the mount @inode was found from
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6867512907d6..9afb30606373 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> kgid_has_mapping(fs_userns, kgid);
> }
>
> -extern struct timespec64 current_time(struct inode *inode);
> +struct timespec64 current_time(struct inode *inode);
> +struct timespec64 inode_ctime_set_current(struct inode *inode);
> +
> +/**
> + * 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)
> +{
> + return inode->i_ctime;
> +}
> +
> +/**
> + * inode_ctime_set - set the ctime in the inode to the given value
> + * @inode: inode in which to set the ctime
> + * @ts: timespec value to set the ctime
> + *
> + * Set the ctime in @inode to @ts.
> + */
> +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
> +{
> + inode->i_ctime = ts;
> + return ts;
> +}
> +
> +/**
> + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
> + * @inode: inode in which to set the ctime
> + * @sec: value to set the tv_sec field
> + *
> + * Set the sec field in the ctime. Returns @sec.
> + */
> +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
> +{
> + inode->i_ctime.tv_sec = sec;
> + return sec;
> +}
> +
> +/**
> + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> + * @inode: inode in which to set the ctime
> + * @nsec: value to set the tv_nsec field
> + *
> + * Set the nsec field in the ctime. Returns @nsec.
> + */
> +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> +{
> + inode->i_ctime.tv_nsec = nsec;
> + return nsec;
> +}
>
> /*
> * Snapshotting support.
> --
> 2.41.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
2023-06-21 16:34 ` Jan Kara
@ 2023-06-21 17:29 ` Tom Talpey
2023-06-21 18:01 ` Jeff Layton
2023-06-22 0:46 ` Damien Le Moal
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Tom Talpey @ 2023-06-21 17:29 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 6/21/2023 10:45 AM, 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>
> ---
> fs/inode.c | 16 ++++++++++++++
> include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index d37fad91c8da..c005e7328fbb 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> }
> EXPORT_SYMBOL(current_time);
>
> +/**
> + * inode_ctime_set_current - set the ctime to current_time
> + * @inode: inode
> + *
> + * Set the inode->i_ctime to the current value for the inode. Returns
> + * the current value that was assigned to i_ctime.
> + */
> +struct timespec64 inode_ctime_set_current(struct inode *inode)
> +{
> + struct timespec64 now = current_time(inode);
> +
> + inode_set_ctime(inode, now);
> + return now;
> +}
> +EXPORT_SYMBOL(inode_ctime_set_current);
> +
> /**
> * in_group_or_capable - check whether caller is CAP_FSETID privileged
> * @idmap: idmap of the mount @inode was found from
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6867512907d6..9afb30606373 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> kgid_has_mapping(fs_userns, kgid);
> }
>
> -extern struct timespec64 current_time(struct inode *inode);
> +struct timespec64 current_time(struct inode *inode);
> +struct timespec64 inode_ctime_set_current(struct inode *inode);
> +
> +/**
> + * 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)
> +{
> + return inode->i_ctime;
> +}
> +
> +/**
> + * inode_ctime_set - set the ctime in the inode to the given value
> + * @inode: inode in which to set the ctime
> + * @ts: timespec value to set the ctime
> + *
> + * Set the ctime in @inode to @ts.
> + */
> +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
> +{
> + inode->i_ctime = ts;
> + return ts;
> +}
> +
> +/**
> + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
I'm curious about why you choose to split the tv_sec and tv_nsec
set_ functions. Do any callers not set them both? Wouldn't a
single call enable a more atomic behavior someday?
inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
(or simply initialize a timespec64 and use inode_ctime_spec() )
Tom.
> + * @inode: inode in which to set the ctime
> + * @sec: value to set the tv_sec field
> + *
> + * Set the sec field in the ctime. Returns @sec.
> + */
> +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
> +{
> + inode->i_ctime.tv_sec = sec;
> + return sec;
> +}
> +
> +/**
> + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> + * @inode: inode in which to set the ctime
> + * @nsec: value to set the tv_nsec field
> + *
> + * Set the nsec field in the ctime. Returns @nsec.
> + */
> +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> +{
> + inode->i_ctime.tv_nsec = nsec;
> + return nsec;
> +}
>
> /*
> * Snapshotting support.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 17:29 ` Tom Talpey
@ 2023-06-21 18:01 ` Jeff Layton
2023-06-21 18:19 ` Tom Talpey
0 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 18:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
> On 6/21/2023 10:45 AM, 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>
> > ---
> > fs/inode.c | 16 ++++++++++++++
> > include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/inode.c b/fs/inode.c
> > index d37fad91c8da..c005e7328fbb 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> > }
> > EXPORT_SYMBOL(current_time);
> >
> > +/**
> > + * inode_ctime_set_current - set the ctime to current_time
> > + * @inode: inode
> > + *
> > + * Set the inode->i_ctime to the current value for the inode. Returns
> > + * the current value that was assigned to i_ctime.
> > + */
> > +struct timespec64 inode_ctime_set_current(struct inode *inode)
> > +{
> > + struct timespec64 now = current_time(inode);
> > +
> > + inode_set_ctime(inode, now);
> > + return now;
> > +}
> > +EXPORT_SYMBOL(inode_ctime_set_current);
> > +
> > /**
> > * in_group_or_capable - check whether caller is CAP_FSETID privileged
> > * @idmap: idmap of the mount @inode was found from
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 6867512907d6..9afb30606373 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> > kgid_has_mapping(fs_userns, kgid);
> > }
> >
> > -extern struct timespec64 current_time(struct inode *inode);
> > +struct timespec64 current_time(struct inode *inode);
> > +struct timespec64 inode_ctime_set_current(struct inode *inode);
> > +
> > +/**
> > + * 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)
> > +{
> > + return inode->i_ctime;
> > +}
> > +
> > +/**
> > + * inode_ctime_set - set the ctime in the inode to the given value
> > + * @inode: inode in which to set the ctime
> > + * @ts: timespec value to set the ctime
> > + *
> > + * Set the ctime in @inode to @ts.
> > + */
> > +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
> > +{
> > + inode->i_ctime = ts;
> > + return ts;
> > +}
> > +
> > +/**
> > + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
>
> I'm curious about why you choose to split the tv_sec and tv_nsec
> set_ functions. Do any callers not set them both? Wouldn't a
> single call enable a more atomic behavior someday?
>
> inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
>
> (or simply initialize a timespec64 and use inode_ctime_spec() )
>
Yes, quite a few places set the fields individually. For example, when
loading a value from disk that doesn't have sufficient granularity to
set the nsecs field to anything but 0.
Could I have done it by declaring a local timespec64 variable and just
use the inode_ctime_set function in these places? Absolutely.
That's a bit more difficult to handle with coccinelle though. If someone
wants to suggest a way to do that without having to change all of these
call sites manually, then I'm open to redoing the set.
That might be better left for a later cleanup though.
> > + * @inode: inode in which to set the ctime
> > + * @sec: value to set the tv_sec field
> > + *
> > + * Set the sec field in the ctime. Returns @sec.
> > + */
> > +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
> > +{
> > + inode->i_ctime.tv_sec = sec;
> > + return sec;
> > +}
> > +
> > +/**
> > + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> > + * @inode: inode in which to set the ctime
> > + * @nsec: value to set the tv_nsec field
> > + *
> > + * Set the nsec field in the ctime. Returns @nsec.
> > + */
> > +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> > +{
> > + inode->i_ctime.tv_nsec = nsec;
> > + return nsec;
> > +}
> >
> > /*
> > * Snapshotting support.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 18:01 ` Jeff Layton
@ 2023-06-21 18:19 ` Tom Talpey
2023-06-21 18:48 ` Jeff Layton
0 siblings, 1 reply; 17+ messages in thread
From: Tom Talpey @ 2023-06-21 18:19 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 6/21/2023 2:01 PM, Jeff Layton wrote:
> On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
>> On 6/21/2023 10:45 AM, 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>
>>> ---
>>> fs/inode.c | 16 ++++++++++++++
>>> include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 68 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/inode.c b/fs/inode.c
>>> index d37fad91c8da..c005e7328fbb 100644
>>> --- a/fs/inode.c
>>> +++ b/fs/inode.c
>>> @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
>>> }
>>> EXPORT_SYMBOL(current_time);
>>>
>>> +/**
>>> + * inode_ctime_set_current - set the ctime to current_time
>>> + * @inode: inode
>>> + *
>>> + * Set the inode->i_ctime to the current value for the inode. Returns
>>> + * the current value that was assigned to i_ctime.
>>> + */
>>> +struct timespec64 inode_ctime_set_current(struct inode *inode)
>>> +{
>>> + struct timespec64 now = current_time(inode);
>>> +
>>> + inode_set_ctime(inode, now);
>>> + return now;
>>> +}
>>> +EXPORT_SYMBOL(inode_ctime_set_current);
>>> +
>>> /**
>>> * in_group_or_capable - check whether caller is CAP_FSETID privileged
>>> * @idmap: idmap of the mount @inode was found from
>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>> index 6867512907d6..9afb30606373 100644
>>> --- a/include/linux/fs.h
>>> +++ b/include/linux/fs.h
>>> @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
>>> kgid_has_mapping(fs_userns, kgid);
>>> }
>>>
>>> -extern struct timespec64 current_time(struct inode *inode);
>>> +struct timespec64 current_time(struct inode *inode);
>>> +struct timespec64 inode_ctime_set_current(struct inode *inode);
>>> +
>>> +/**
>>> + * 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)
>>> +{
>>> + return inode->i_ctime;
>>> +}
>>> +
>>> +/**
>>> + * inode_ctime_set - set the ctime in the inode to the given value
>>> + * @inode: inode in which to set the ctime
>>> + * @ts: timespec value to set the ctime
>>> + *
>>> + * Set the ctime in @inode to @ts.
>>> + */
>>> +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
>>> +{
>>> + inode->i_ctime = ts;
>>> + return ts;
>>> +}
>>> +
>>> +/**
>>> + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
>>
>> I'm curious about why you choose to split the tv_sec and tv_nsec
>> set_ functions. Do any callers not set them both? Wouldn't a
>> single call enable a more atomic behavior someday?
>>
>> inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
>>
>> (or simply initialize a timespec64 and use inode_ctime_spec() )
>>
>
> Yes, quite a few places set the fields individually. For example, when
> loading a value from disk that doesn't have sufficient granularity to
> set the nsecs field to anything but 0.
Well, they still need to set the tv_nsec so they could just pass 0.
But ok.
> Could I have done it by declaring a local timespec64 variable and just
> use the inode_ctime_set function in these places? Absolutely.
>
> That's a bit more difficult to handle with coccinelle though. If someone
> wants to suggest a way to do that without having to change all of these
> call sites manually, then I'm open to redoing the set.
>
> That might be better left for a later cleanup though.
Acked-by: Tom Talpey <tom@talpey.com>
>>> + * @inode: inode in which to set the ctime
>>> + * @sec: value to set the tv_sec field
>>> + *
>>> + * Set the sec field in the ctime. Returns @sec.
>>> + */
>>> +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
>>> +{
>>> + inode->i_ctime.tv_sec = sec;
>>> + return sec;
>>> +}
>>> +
>>> +/**
>>> + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
>>> + * @inode: inode in which to set the ctime
>>> + * @nsec: value to set the tv_nsec field
>>> + *
>>> + * Set the nsec field in the ctime. Returns @nsec.
>>> + */
>>> +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
>>> +{
>>> + inode->i_ctime.tv_nsec = nsec;
>>> + return nsec;
>>> +}
>>>
>>> /*
>>> * Snapshotting support.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 18:19 ` Tom Talpey
@ 2023-06-21 18:48 ` Jeff Layton
0 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 18:48 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Wed, 2023-06-21 at 14:19 -0400, Tom Talpey wrote:
> On 6/21/2023 2:01 PM, Jeff Layton wrote:
> > On Wed, 2023-06-21 at 13:29 -0400, Tom Talpey wrote:
> > > On 6/21/2023 10:45 AM, 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>
> > > > ---
> > > > fs/inode.c | 16 ++++++++++++++
> > > > include/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++++++++++++-
> > > > 2 files changed, 68 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/inode.c b/fs/inode.c
> > > > index d37fad91c8da..c005e7328fbb 100644
> > > > --- a/fs/inode.c
> > > > +++ b/fs/inode.c
> > > > @@ -2499,6 +2499,22 @@ struct timespec64 current_time(struct inode *inode)
> > > > }
> > > > EXPORT_SYMBOL(current_time);
> > > >
> > > > +/**
> > > > + * inode_ctime_set_current - set the ctime to current_time
> > > > + * @inode: inode
> > > > + *
> > > > + * Set the inode->i_ctime to the current value for the inode. Returns
> > > > + * the current value that was assigned to i_ctime.
> > > > + */
> > > > +struct timespec64 inode_ctime_set_current(struct inode *inode)
> > > > +{
> > > > + struct timespec64 now = current_time(inode);
> > > > +
> > > > + inode_set_ctime(inode, now);
> > > > + return now;
> > > > +}
> > > > +EXPORT_SYMBOL(inode_ctime_set_current);
> > > > +
> > > > /**
> > > > * in_group_or_capable - check whether caller is CAP_FSETID privileged
> > > > * @idmap: idmap of the mount @inode was found from
> > > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > > index 6867512907d6..9afb30606373 100644
> > > > --- a/include/linux/fs.h
> > > > +++ b/include/linux/fs.h
> > > > @@ -1474,7 +1474,58 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> > > > kgid_has_mapping(fs_userns, kgid);
> > > > }
> > > >
> > > > -extern struct timespec64 current_time(struct inode *inode);
> > > > +struct timespec64 current_time(struct inode *inode);
> > > > +struct timespec64 inode_ctime_set_current(struct inode *inode);
> > > > +
> > > > +/**
> > > > + * 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)
> > > > +{
> > > > + return inode->i_ctime;
> > > > +}
> > > > +
> > > > +/**
> > > > + * inode_ctime_set - set the ctime in the inode to the given value
> > > > + * @inode: inode in which to set the ctime
> > > > + * @ts: timespec value to set the ctime
> > > > + *
> > > > + * Set the ctime in @inode to @ts.
> > > > + */
> > > > +static inline struct timespec64 inode_ctime_set(struct inode *inode, struct timespec64 ts)
> > > > +{
> > > > + inode->i_ctime = ts;
> > > > + return ts;
> > > > +}
> > > > +
> > > > +/**
> > > > + * inode_ctime_set_sec - set only the tv_sec field in the inode ctime
> > >
> > > I'm curious about why you choose to split the tv_sec and tv_nsec
> > > set_ functions. Do any callers not set them both? Wouldn't a
> > > single call enable a more atomic behavior someday?
> > >
> > > inode_ctime_set_sec_nsec(struct inode *, time64_t, time64_t)
> > >
> > > (or simply initialize a timespec64 and use inode_ctime_spec() )
> > >
> >
> > Yes, quite a few places set the fields individually. For example, when
> > loading a value from disk that doesn't have sufficient granularity to
> > set the nsecs field to anything but 0.
>
> Well, they still need to set the tv_nsec so they could just pass 0.
> But ok.
>
Sure. The difficulty is in trying to do this in an automated way. For
instance, look at the hfsplus patch; it has separate assignments in
place already:
- result->i_ctime.tv_sec = result->i_mtime.tv_sec = result->i_atime.tv_sec = local_to_gmt(dir->i_sb, le32_to_cpu(dee.creation_date));
- result->i_ctime.tv_nsec = 0;
+ inode_ctime_set_sec(result,
+ result->i_mtime.tv_sec = result->i_atime.tv_sec = local_to_gmt(dir->i_sb, le32_to_cpu(dee.creation_date)));
+ inode_ctime_set_nsec(result, 0);
Granted the new code is pretty ugly, but it compiles!
Transforming that into what you're suggesting is a tougher proposition
to do with coccinelle. I didn't see a way to conditionally catch cases
like this, declare a new variable in the appropriate spot and then
transform two assignments (that may not be next to one another!) into a
single one.
Maybe it's possible, but my grasp of SMPL is not that great. The docs
and examples (including Kees' vey helpful ones!) cover fairly simple
changes well, but I didn't quite grasp how to do that complex an
evolution.
> > Could I have done it by declaring a local timespec64 variable and just
> > use the inode_ctime_set function in these places? Absolutely.
> >
> > That's a bit more difficult to handle with coccinelle though. If someone
> > wants to suggest a way to do that without having to change all of these
> > call sites manually, then I'm open to redoing the set.
> >
> > That might be better left for a later cleanup though.
>
> Acked-by: Tom Talpey <tom@talpey.com>
>
Many thanks!
> > > > + * @inode: inode in which to set the ctime
> > > > + * @sec: value to set the tv_sec field
> > > > + *
> > > > + * Set the sec field in the ctime. Returns @sec.
> > > > + */
> > > > +static inline time64_t inode_ctime_set_sec(struct inode *inode, time64_t sec)
> > > > +{
> > > > + inode->i_ctime.tv_sec = sec;
> > > > + return sec;
> > > > +}
> > > > +
> > > > +/**
> > > > + * inode_ctime_set_nsec - set only the tv_nsec field in the inode ctime
> > > > + * @inode: inode in which to set the ctime
> > > > + * @nsec: value to set the tv_nsec field
> > > > + *
> > > > + * Set the nsec field in the ctime. Returns @nsec.
> > > > + */
> > > > +static inline long inode_ctime_set_nsec(struct inode *inode, long nsec)
> > > > +{
> > > > + inode->i_ctime.tv_nsec = nsec;
> > > > + return nsec;
> > > > +}
> > > >
> > > > /*
> > > > * Snapshotting support.
> >
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime
2023-06-21 14:45 [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Jeff Layton
` (2 preceding siblings ...)
2023-06-21 14:49 ` [Cluster-devel] [PATCH 79/79] fs: rename i_ctime field to __i_ctime Jeff Layton
@ 2023-06-21 19:21 ` Steven Rostedt
2023-06-21 19:52 ` Jeff Layton
2023-06-30 22:11 ` Luis Chamberlain
3 siblings, 2 replies; 17+ messages in thread
From: Steven Rostedt @ 2023-06-21 19:21 UTC (permalink / raw)
To: cluster-devel.redhat.com
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.
-- Steve
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime
2023-06-21 19:21 ` [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Steven Rostedt
@ 2023-06-21 19:52 ` Jeff Layton
2023-06-23 12:41 ` Christian Brauner
2023-06-30 22:11 ` Luis Chamberlain
1 sibling, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2023-06-21 19:52 UTC (permalink / raw)
To: cluster-devel.redhat.com
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)?
------------------------8<------------------------------
@@
expression inode;
@@
- inode->i_ctime = current_time(inode)
+ inode_set_current_ctime(inode)
@@
expression inode;
@@
- inode->i_ctime = inode->i_mtime = current_time(inode)
+ inode->i_mtime = inode_set_current_ctime(inode)
@@
struct inode *inode;
expression value;
@@
- inode->i_ctime = value;
+ inode_set_ctime(inode, value);
@@
struct inode *inode;
expression val;
@@
- inode->i_ctime.tv_sec = val
+ inode_set_ctime_sec(inode, val)
@@
struct inode *inode;
expression val;
@@
- inode->i_ctime.tv_nsec = val
+ inode_set_ctime_nsec(inode, val)
@@
struct inode *inode;
@@
- inode->i_ctime
+ inode_ctime_peek(inode)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
2023-06-21 16:34 ` Jan Kara
2023-06-21 17:29 ` Tom Talpey
@ 2023-06-22 0:46 ` Damien Le Moal
2023-06-22 10:14 ` Jeff Layton
2023-06-30 22:12 ` Luis Chamberlain
2023-07-12 15:31 ` Randy Dunlap
4 siblings, 1 reply; 17+ messages in thread
From: Damien Le Moal @ 2023-06-22 0:46 UTC (permalink / raw)
To: cluster-devel.redhat.com
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()
? Also, inode_set_ctime() & inode_get_ctime() may be a little more natural. But
no strong opinion about that though.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-22 0:46 ` Damien Le Moal
@ 2023-06-22 10:14 ` Jeff Layton
0 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2023-06-22 10:14 UTC (permalink / raw)
To: cluster-devel.redhat.com
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime
2023-06-21 19:52 ` Jeff Layton
@ 2023-06-23 12:41 ` Christian Brauner
0 siblings, 0 replies; 17+ messages in thread
From: Christian Brauner @ 2023-06-23 12:41 UTC (permalink / raw)
To: cluster-devel.redhat.com
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime
2023-06-21 19:21 ` [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Steven Rostedt
2023-06-21 19:52 ` Jeff Layton
@ 2023-06-30 22:11 ` Luis Chamberlain
1 sibling, 0 replies; 17+ messages in thread
From: Luis Chamberlain @ 2023-06-30 22:11 UTC (permalink / raw)
To: cluster-devel.redhat.com
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 at twin.jikos.cz/
Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
` (2 preceding siblings ...)
2023-06-22 0:46 ` Damien Le Moal
@ 2023-06-30 22:12 ` Luis Chamberlain
2023-07-12 15:31 ` Randy Dunlap
4 siblings, 0 replies; 17+ messages in thread
From: Luis Chamberlain @ 2023-06-30 22:12 UTC (permalink / raw)
To: cluster-devel.redhat.com
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
` (3 preceding siblings ...)
2023-06-30 22:12 ` Luis Chamberlain
@ 2023-07-12 15:31 ` Randy Dunlap
4 siblings, 0 replies; 17+ messages in thread
From: Randy Dunlap @ 2023-07-12 15:31 UTC (permalink / raw)
To: cluster-devel.redhat.com
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-07-12 15:31 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 14:45 [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Jeff Layton
2023-06-21 14:45 ` [Cluster-devel] [PATCH 01/79] fs: add ctime accessors infrastructure Jeff Layton
2023-06-21 16:34 ` Jan Kara
2023-06-21 17:29 ` Tom Talpey
2023-06-21 18:01 ` Jeff Layton
2023-06-21 18:19 ` Tom Talpey
2023-06-21 18:48 ` Jeff Layton
2023-06-22 0:46 ` Damien Le Moal
2023-06-22 10:14 ` Jeff Layton
2023-06-30 22:12 ` Luis Chamberlain
2023-07-12 15:31 ` Randy Dunlap
[not found] ` <20230621144735.55953-1-jlayton@kernel.org>
2023-06-21 14:45 ` [Cluster-devel] [PATCH 34/79] gfs2: switch to new ctime accessors Jeff Layton
2023-06-21 14:49 ` [Cluster-devel] [PATCH 79/79] fs: rename i_ctime field to __i_ctime Jeff Layton
2023-06-21 19:21 ` [Cluster-devel] [PATCH 00/79] fs: new accessors for inode->i_ctime Steven Rostedt
2023-06-21 19:52 ` Jeff Layton
2023-06-23 12:41 ` Christian Brauner
2023-06-30 22:11 ` Luis Chamberlain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).