* [PATCH 00/87] fs: new accessor methods for atime and mtime
@ 2023-09-28 11:02 Jeff Layton
2023-09-28 11:03 ` [PATCH 01/87] " Jeff Layton
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
0 siblings, 2 replies; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 11:02 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Linus Torvalds, David Sterba,
Amir Goldstein, Theodore Ts'o, Eric Biederman, Kees Cook,
Jeremy Kerr, Arnd Bergmann, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Carlos Llamas, Suren Baghdasaryan,
Mattia Dongili, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Hans de Goede,
Ilpo Järvinen, Mark Gross, Jiri Slaby, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian, Chris Mason,
Josef Bacik, Xiubo Li, Ilya Dryomov, Jan Harkes, coda,
Joel Becker, Christoph Hellwig, Nicolas Pitre, Rafael J. Wysocki,
Ard Biesheuvel, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Namjae Jeon, Sungjong Seo, Jan Kara, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Christoph Hellwig, Miklos Szeredi, Bob Peterson,
Andreas Gruenbacher, Richard Weinberger, Anton Ivanov,
Johannes Berg, Mikulas Patocka, Mike Kravetz, Muchun Song,
Jan Kara, David Woodhouse, Dave Kleikamp, Tejun Heo,
Trond Myklebust, Anna Schumaker, Chuck Lever, Neil Brown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Ryusuke Konishi,
Anton Altaparmakov, Konstantin Komarov, Mark Fasheh, Joseph Qi,
Bob Copeland, Mike Marshall, Martin Brandenburg, Luis Chamberlain,
Iurii Zaikin, Tony Luck, Guilherme G. Piccoli, Anders Larsen,
Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Sergey Senozhatsky, Phillip Lougher, Steven Rostedt,
Masami Hiramatsu, Evgeniy Dushistov, Chandan Babu R,
Darrick J. Wong, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Hugh Dickins,
Andrew Morton, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Eric Paris
Cc: linux-fsdevel, linux-kernel, linux-mm, linuxppc-dev, linux-s390,
platform-driver-x86, linux-rdma, linux-serial, linux-usb, v9fs,
linux-afs, autofs, linux-btrfs, ceph-devel, codalist, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, gfs2, linux-um,
linux-mtd, jfs-discussion, linux-nfs, linux-nilfs, linux-ntfs-dev,
ntfs3, ocfs2-devel, linux-karma-devel, devel, linux-unionfs,
linux-hardening, reiserfs-devel, linux-cifs, samba-technical,
linux-trace-kernel, linux-xfs, bpf, netdev, apparmor,
linux-security-module, selinux
While working on the multigrain timestamp changes, Linus suggested
adding some similar wrappers for accessing the atime and mtime that we
have for the ctime. With that, we could then move to using discrete
integers instead of timespec64 in struct inode, and shrink it.
Linus suggested using macros for the new accessors, but the existing
ctime wrappers were static inlines and since there are only 3 different
timestamps, I didn't see that trying to fiddle with macros would gain us
anything.
The first patches start with some new infrastructure, and then we move
to converting different subsystems to use it. The second to last patch
makes the conversion to discrete integers, which shaves 8 bytes off of
struct inode on my x86_64 kernel. The last patch reshuffles things a
bit more, to keep the i_lock in the same cacheline as the fields it
protects (at least on x86_64).
About 75% of this conversion was done with coccinelle, with the rest
done by hand with vim.
Jeff Layton (87):
fs: new accessor methods for atime and mtime
fs: convert core infrastructure to new {a,m}time accessors
arch/powerpc/platforms/cell/spufs: convert to new inode {a,m}time
accessors
arch/s390/hypfs: convert to new inode {a,m}time accessors
drivers/android: convert to new inode {a,m}time accessors
drivers/char: convert to new inode {a,m}time accessors
drivers/infiniband/hw/qib: convert to new inode {a,m}time accessors
drivers/misc/ibmasm: convert to new inode {a,m}time accessors
drivers/misc: convert to new inode {a,m}time accessors
drivers/platform/x86: convert to new inode {a,m}time accessors
drivers/tty: convert to new inode {a,m}time accessors
drivers/usb/core: convert to new inode {a,m}time accessors
drivers/usb/gadget/function: convert to new inode {a,m}time accessors
drivers/usb/gadget/legacy: convert to new inode {a,m}time accessors
fs/9p: convert to new inode {a,m}time accessors
fs/adfs: convert to new inode {a,m}time accessors
fs/affs: convert to new inode {a,m}time accessors
fs/afs: convert to new inode {a,m}time accessors
fs/autofs: convert to new inode {a,m}time accessors
fs/befs: convert to new inode {a,m}time accessors
fs/bfs: convert to new inode {a,m}time accessors
fs/btrfs: convert to new inode {a,m}time accessors
fs/ceph: convert to new inode {a,m}time accessors
fs/coda: convert to new inode {a,m}time accessors
fs/configfs: convert to new inode {a,m}time accessors
fs/cramfs: convert to new inode {a,m}time accessors
fs/debugfs: convert to new inode {a,m}time accessors
fs/devpts: convert to new inode {a,m}time accessors
fs/efivarfs: convert to new inode {a,m}time accessors
fs/efs: convert to new inode {a,m}time accessors
fs/erofs: convert to new inode {a,m}time accessors
fs/exfat: convert to new inode {a,m}time accessors
fs/ext2: convert to new inode {a,m}time accessors
fs/ext4: convert to new inode {a,m}time accessors
fs/f2fs: convert to new inode {a,m}time accessors
fs/fat: convert to new inode {a,m}time accessors
fs/freevxfs: convert to new inode {a,m}time accessors
fs/fuse: convert to new inode {a,m}time accessors
fs/gfs2: convert to new inode {a,m}time accessors
fs/hfs: convert to new inode {a,m}time accessors
fs/hfsplus: convert to new inode {a,m}time accessors
fs/hostfs: convert to new inode {a,m}time accessors
fs/hpfs: convert to new inode {a,m}time accessors
fs/hugetlbfs: convert to new inode {a,m}time accessors
fs/isofs: convert to new inode {a,m}time accessors
fs/jffs2: convert to new inode {a,m}time accessors
fs/jfs: convert to new inode {a,m}time accessors
fs/kernfs: convert to new inode {a,m}time accessors
fs/minix: convert to new inode {a,m}time accessors
fs/nfs: convert to new inode {a,m}time accessors
fs/nfsd: convert to new inode {a,m}time accessors
fs/nilfs2: convert to new inode {a,m}time accessors
fs/ntfs: convert to new inode {a,m}time accessors
fs/ntfs3: convert to new inode {a,m}time accessors
fs/ocfs2: convert to new inode {a,m}time accessors
fs/omfs: convert to new inode {a,m}time accessors
fs/openpromfs: convert to new inode {a,m}time accessors
fs/orangefs: convert to new inode {a,m}time accessors
fs/overlayfs: convert to new inode {a,m}time accessors
fs/proc: convert to new inode {a,m}time accessors
fs/pstore: convert to new inode {a,m}time accessors
fs/qnx4: convert to new inode {a,m}time accessors
fs/qnx6: convert to new inode {a,m}time accessors
fs/ramfs: convert to new inode {a,m}time accessors
fs/reiserfs: convert to new inode {a,m}time accessors
fs/romfs: convert to new inode {a,m}time accessors
fs/smb/client: convert to new inode {a,m}time accessors
fs/smb/server: convert to new inode {a,m}time accessors
fs/squashfs: convert to new inode {a,m}time accessors
fs/sysv: convert to new inode {a,m}time accessors
fs/tracefs: convert to new inode {a,m}time accessors
fs/ubifs: convert to new inode {a,m}time accessors
fs/udf: convert to new inode {a,m}time accessors
fs/ufs: convert to new inode {a,m}time accessors
fs/vboxsf: convert to new inode {a,m}time accessors
fs/xfs: convert to new inode {a,m}time accessors
fs/zonefs: convert to new inode {a,m}time accessors
ipc: convert to new inode {a,m}time accessors
kernel/bpf: convert to new inode {a,m}time accessors
mm: convert to new inode {a,m}time accessors
net/sunrpc: convert to new inode {a,m}time accessors
security/apparmor: convert to new inode {a,m}time accessors
security/selinux: convert to new inode {a,m}time accessors
security: convert to new inode {a,m}time accessors
fs: rename i_atime and i_mtime fields to __i_atime and __i_mtime
fs: switch timespec64 fields in inode to discrete integers
fs: move i_blocks up a few places in struct inode
arch/powerpc/platforms/cell/spufs/inode.c | 2 +-
arch/s390/hypfs/inode.c | 4 +-
drivers/android/binderfs.c | 8 +--
drivers/char/sonypi.c | 2 +-
drivers/infiniband/hw/qib/qib_fs.c | 4 +-
drivers/misc/ibmasm/ibmasmfs.c | 2 +-
drivers/misc/ibmvmc.c | 2 +-
drivers/platform/x86/sony-laptop.c | 2 +-
drivers/tty/tty_io.c | 10 +++-
drivers/usb/core/devio.c | 26 ++++++---
drivers/usb/gadget/function/f_fs.c | 4 +-
drivers/usb/gadget/legacy/inode.c | 2 +-
fs/9p/vfs_inode.c | 6 +-
fs/9p/vfs_inode_dotl.c | 16 +++---
fs/adfs/inode.c | 13 +++--
fs/affs/amigaffs.c | 4 +-
fs/affs/inode.c | 17 +++---
fs/afs/dynroot.c | 2 +-
fs/afs/inode.c | 8 +--
fs/afs/write.c | 2 +-
fs/attr.c | 4 +-
fs/autofs/inode.c | 2 +-
fs/autofs/root.c | 6 +-
fs/bad_inode.c | 2 +-
fs/befs/linuxvfs.c | 10 ++--
fs/bfs/dir.c | 9 +--
fs/bfs/inode.c | 10 ++--
fs/binfmt_misc.c | 2 +-
fs/btrfs/delayed-inode.c | 16 +++---
fs/btrfs/file.c | 18 +++---
fs/btrfs/inode.c | 39 ++++++-------
fs/btrfs/reflink.c | 2 +-
fs/btrfs/transaction.c | 3 +-
fs/btrfs/tree-log.c | 8 +--
fs/ceph/addr.c | 10 ++--
fs/ceph/caps.c | 4 +-
fs/ceph/file.c | 2 +-
fs/ceph/inode.c | 60 +++++++++++---------
fs/ceph/mds_client.c | 8 ++-
fs/ceph/snap.c | 4 +-
fs/coda/coda_linux.c | 6 +-
fs/coda/dir.c | 2 +-
fs/coda/file.c | 2 +-
fs/configfs/inode.c | 8 +--
fs/cramfs/inode.c | 4 +-
fs/debugfs/inode.c | 2 +-
fs/devpts/inode.c | 6 +-
fs/efivarfs/file.c | 2 +-
fs/efivarfs/inode.c | 2 +-
fs/efs/inode.c | 5 +-
fs/erofs/inode.c | 3 +-
fs/exfat/exfat_fs.h | 1 +
fs/exfat/file.c | 7 +--
fs/exfat/inode.c | 31 ++++++-----
fs/exfat/misc.c | 8 +++
fs/exfat/namei.c | 31 ++++++-----
fs/exfat/super.c | 4 +-
fs/ext2/dir.c | 6 +-
fs/ext2/ialloc.c | 2 +-
fs/ext2/inode.c | 11 ++--
fs/ext2/super.c | 2 +-
fs/ext4/ext4.h | 20 +++++--
fs/ext4/extents.c | 11 ++--
fs/ext4/ialloc.c | 4 +-
fs/ext4/inline.c | 4 +-
fs/ext4/inode.c | 19 ++++---
fs/ext4/ioctl.c | 13 ++++-
fs/ext4/namei.c | 10 ++--
fs/ext4/super.c | 2 +-
fs/ext4/xattr.c | 6 +-
fs/f2fs/dir.c | 6 +-
fs/f2fs/f2fs.h | 10 ++--
fs/f2fs/file.c | 14 ++---
fs/f2fs/inline.c | 2 +-
fs/f2fs/inode.c | 20 +++----
fs/f2fs/namei.c | 4 +-
fs/f2fs/recovery.c | 8 +--
fs/f2fs/super.c | 2 +-
fs/fat/inode.c | 25 ++++++---
fs/fat/misc.c | 6 +-
fs/freevxfs/vxfs_inode.c | 6 +-
fs/fuse/control.c | 2 +-
fs/fuse/dir.c | 6 +-
fs/fuse/inode.c | 25 ++++-----
fs/fuse/readdir.c | 6 +-
fs/gfs2/bmap.c | 10 ++--
fs/gfs2/dir.c | 10 ++--
fs/gfs2/glops.c | 11 ++--
fs/gfs2/inode.c | 7 ++-
fs/gfs2/quota.c | 2 +-
fs/gfs2/super.c | 8 +--
fs/hfs/catalog.c | 8 +--
fs/hfs/inode.c | 16 +++---
fs/hfs/sysdep.c | 10 ++--
fs/hfsplus/catalog.c | 8 +--
fs/hfsplus/inode.c | 22 ++++----
fs/hostfs/hostfs_kern.c | 12 ++--
fs/hpfs/dir.c | 10 ++--
fs/hpfs/inode.c | 12 ++--
fs/hpfs/namei.c | 20 +++----
fs/hpfs/super.c | 10 ++--
fs/hugetlbfs/inode.c | 10 ++--
fs/inode.c | 35 +++++++-----
fs/isofs/inode.c | 4 +-
fs/isofs/rock.c | 18 +++---
fs/jffs2/dir.c | 35 +++++++-----
fs/jffs2/file.c | 4 +-
fs/jffs2/fs.c | 20 +++----
fs/jffs2/os-linux.h | 4 +-
fs/jfs/inode.c | 2 +-
fs/jfs/jfs_imap.c | 16 +++---
fs/jfs/jfs_inode.c | 2 +-
fs/jfs/namei.c | 20 ++++---
fs/jfs/super.c | 2 +-
fs/kernfs/inode.c | 6 +-
fs/libfs.c | 41 ++++++++++----
fs/minix/bitmap.c | 2 +-
fs/minix/dir.c | 6 +-
fs/minix/inode.c | 15 +++--
fs/minix/itree_common.c | 2 +-
fs/nfs/callback_proc.c | 2 +-
fs/nfs/fscache.h | 4 +-
fs/nfs/inode.c | 30 +++++-----
fs/nfsd/blocklayout.c | 3 +-
fs/nfsd/nfs3proc.c | 4 +-
fs/nfsd/nfs4proc.c | 8 +--
fs/nfsd/nfsctl.c | 2 +-
fs/nilfs2/dir.c | 6 +-
fs/nilfs2/inode.c | 16 +++---
fs/nsfs.c | 2 +-
fs/ntfs/inode.c | 25 +++++----
fs/ntfs/mft.c | 2 +-
fs/ntfs3/file.c | 6 +-
fs/ntfs3/frecord.c | 11 ++--
fs/ntfs3/inode.c | 22 +++++---
fs/ntfs3/namei.c | 4 +-
fs/ocfs2/alloc.c | 2 +-
fs/ocfs2/aops.c | 6 +-
fs/ocfs2/dir.c | 5 +-
fs/ocfs2/dlmfs/dlmfs.c | 4 +-
fs/ocfs2/dlmglue.c | 29 +++++-----
fs/ocfs2/file.c | 26 +++++----
fs/ocfs2/inode.c | 24 ++++----
fs/ocfs2/namei.c | 8 +--
fs/ocfs2/refcounttree.c | 4 +-
fs/omfs/inode.c | 8 +--
fs/openpromfs/inode.c | 4 +-
fs/orangefs/orangefs-utils.c | 16 +++---
fs/overlayfs/file.c | 9 ++-
fs/overlayfs/inode.c | 3 +-
fs/overlayfs/util.c | 4 +-
fs/pipe.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 | 5 +-
fs/qnx4/inode.c | 6 +-
fs/qnx6/inode.c | 6 +-
fs/ramfs/inode.c | 7 ++-
fs/reiserfs/inode.c | 22 +++-----
fs/reiserfs/namei.c | 8 +--
fs/reiserfs/stree.c | 5 +-
fs/reiserfs/super.c | 2 +-
fs/romfs/super.c | 3 +-
fs/smb/client/file.c | 18 +++---
fs/smb/client/fscache.h | 6 +-
fs/smb/client/inode.c | 17 +++---
fs/smb/client/smb2ops.c | 6 +-
fs/smb/server/smb2pdu.c | 8 +--
fs/squashfs/inode.c | 6 +-
fs/stack.c | 4 +-
fs/stat.c | 4 +-
fs/sysv/dir.c | 6 +-
fs/sysv/ialloc.c | 2 +-
fs/sysv/inode.c | 10 ++--
fs/sysv/itree.c | 2 +-
fs/tracefs/inode.c | 2 +-
fs/ubifs/debug.c | 8 +--
fs/ubifs/dir.c | 23 +++++---
fs/ubifs/file.c | 16 +++---
fs/ubifs/journal.c | 8 +--
fs/ubifs/super.c | 8 +--
fs/udf/ialloc.c | 4 +-
fs/udf/inode.c | 38 +++++++------
fs/udf/namei.c | 16 +++---
fs/ufs/dir.c | 6 +-
fs/ufs/ialloc.c | 2 +-
fs/ufs/inode.c | 36 +++++++-----
fs/vboxsf/utils.c | 15 ++---
fs/xfs/libxfs/xfs_inode_buf.c | 10 ++--
fs/xfs/libxfs/xfs_rtbitmap.c | 6 +-
fs/xfs/libxfs/xfs_trans_inode.c | 2 +-
fs/xfs/xfs_bmap_util.c | 7 ++-
fs/xfs/xfs_inode.c | 4 +-
fs/xfs/xfs_inode_item.c | 4 +-
fs/xfs/xfs_iops.c | 8 +--
fs/xfs/xfs_itable.c | 8 +--
fs/xfs/xfs_rtalloc.c | 30 +++++-----
fs/zonefs/super.c | 10 ++--
include/linux/fs.h | 68 +++++++++++++++++++++--
include/linux/fs_stack.h | 6 +-
ipc/mqueue.c | 19 ++++---
kernel/bpf/inode.c | 5 +-
mm/shmem.c | 20 +++----
net/sunrpc/rpc_pipe.c | 2 +-
security/apparmor/apparmorfs.c | 7 ++-
security/apparmor/policy_unpack.c | 4 +-
security/inode.c | 2 +-
security/selinux/selinuxfs.c | 2 +-
211 files changed, 1115 insertions(+), 906 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 50/87] fs/nfs: convert to new inode {a,m}time accessors
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
@ 2023-09-28 11:02 ` Jeff Layton
2023-09-28 11:03 ` [PATCH 51/87] fs/nfsd: " Jeff Layton
2023-09-28 11:03 ` [PATCH 81/87] net/sunrpc: " Jeff Layton
2 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 11:02 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel; +Cc: linux-nfs
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfs/callback_proc.c | 2 +-
fs/nfs/fscache.h | 4 ++--
fs/nfs/inode.c | 30 +++++++++++++++---------------
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 6bed1394d748..96a4923080ae 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -60,7 +60,7 @@ __be32 nfs4_callback_getattr(void *argp, void *resp,
if (nfs_have_writebacks(inode))
res->change_attr++;
res->ctime = inode_get_ctime(inode);
- res->mtime = inode->i_mtime;
+ res->mtime = inode_get_mtime(inode);
res->bitmap[0] = (FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE) &
args->bitmap[0];
res->bitmap[1] = (FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY) &
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 2dc64454492b..5407ab8c8783 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -114,8 +114,8 @@ static inline void nfs_fscache_update_auxdata(struct nfs_fscache_inode_auxdata *
struct inode *inode)
{
memset(auxdata, 0, sizeof(*auxdata));
- auxdata->mtime_sec = inode->i_mtime.tv_sec;
- auxdata->mtime_nsec = inode->i_mtime.tv_nsec;
+ auxdata->mtime_sec = inode_get_mtime(inode).tv_sec;
+ auxdata->mtime_nsec = inode_get_mtime(inode).tv_nsec;
auxdata->ctime_sec = inode_get_ctime(inode).tv_sec;
auxdata->ctime_nsec = inode_get_ctime(inode).tv_nsec;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index e21c073158e5..ebb8d60e1152 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -512,8 +512,8 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
} else
init_special_inode(inode, inode->i_mode, fattr->rdev);
- memset(&inode->i_atime, 0, sizeof(inode->i_atime));
- memset(&inode->i_mtime, 0, sizeof(inode->i_mtime));
+ inode_set_atime(inode, 0, 0);
+ inode_set_mtime(inode, 0, 0);
inode_set_ctime(inode, 0, 0);
inode_set_iversion_raw(inode, 0);
inode->i_size = 0;
@@ -527,11 +527,11 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
nfsi->read_cache_jiffies = fattr->time_start;
nfsi->attr_gencount = fattr->gencount;
if (fattr->valid & NFS_ATTR_FATTR_ATIME)
- inode->i_atime = fattr->atime;
+ inode_set_atime_to_ts(inode, fattr->atime);
else if (fattr_supported & NFS_ATTR_FATTR_ATIME)
nfs_set_cache_invalid(inode, NFS_INO_INVALID_ATIME);
if (fattr->valid & NFS_ATTR_FATTR_MTIME)
- inode->i_mtime = fattr->mtime;
+ inode_set_mtime_to_ts(inode, fattr->mtime);
else if (fattr_supported & NFS_ATTR_FATTR_MTIME)
nfs_set_cache_invalid(inode, NFS_INO_INVALID_MTIME);
if (fattr->valid & NFS_ATTR_FATTR_CTIME)
@@ -742,9 +742,9 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr,
NFS_I(inode)->cache_validity &= ~(NFS_INO_INVALID_ATIME
| NFS_INO_INVALID_CTIME);
if (fattr->valid & NFS_ATTR_FATTR_ATIME)
- inode->i_atime = fattr->atime;
+ inode_set_atime_to_ts(inode, fattr->atime);
else if (attr->ia_valid & ATTR_ATIME_SET)
- inode->i_atime = attr->ia_atime;
+ inode_set_atime_to_ts(inode, attr->ia_atime);
else
nfs_set_cache_invalid(inode, NFS_INO_INVALID_ATIME);
@@ -758,9 +758,9 @@ void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr,
NFS_I(inode)->cache_validity &= ~(NFS_INO_INVALID_MTIME
| NFS_INO_INVALID_CTIME);
if (fattr->valid & NFS_ATTR_FATTR_MTIME)
- inode->i_mtime = fattr->mtime;
+ inode_set_mtime_to_ts(inode, fattr->mtime);
else if (attr->ia_valid & ATTR_MTIME_SET)
- inode->i_mtime = attr->ia_mtime;
+ inode_set_mtime_to_ts(inode, attr->ia_mtime);
else
nfs_set_cache_invalid(inode, NFS_INO_INVALID_MTIME);
@@ -1451,11 +1451,11 @@ static void nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr)
inode_set_ctime_to_ts(inode, fattr->ctime);
}
- ts = inode->i_mtime;
+ ts = inode_get_mtime(inode);
if ((fattr->valid & NFS_ATTR_FATTR_PREMTIME)
&& (fattr->valid & NFS_ATTR_FATTR_MTIME)
&& timespec64_equal(&ts, &fattr->pre_mtime)) {
- inode->i_mtime = fattr->mtime;
+ inode_set_mtime_to_ts(inode, fattr->mtime);
}
if ((fattr->valid & NFS_ATTR_FATTR_PRESIZE)
&& (fattr->valid & NFS_ATTR_FATTR_SIZE)
@@ -1506,7 +1506,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
if ((fattr->valid & NFS_ATTR_FATTR_CHANGE) != 0 && !inode_eq_iversion_raw(inode, fattr->change_attr))
invalid |= NFS_INO_INVALID_CHANGE;
- ts = inode->i_mtime;
+ ts = inode_get_mtime(inode);
if ((fattr->valid & NFS_ATTR_FATTR_MTIME) && !timespec64_equal(&ts, &fattr->mtime))
invalid |= NFS_INO_INVALID_MTIME;
@@ -1534,7 +1534,7 @@ static int nfs_check_inode_attributes(struct inode *inode, struct nfs_fattr *fat
if ((fattr->valid & NFS_ATTR_FATTR_NLINK) && inode->i_nlink != fattr->nlink)
invalid |= NFS_INO_INVALID_NLINK;
- ts = inode->i_atime;
+ ts = inode_get_atime(inode);
if ((fattr->valid & NFS_ATTR_FATTR_ATIME) && !timespec64_equal(&ts, &fattr->atime))
invalid |= NFS_INO_INVALID_ATIME;
@@ -2002,7 +2002,7 @@ int nfs_post_op_update_inode_force_wcc_locked(struct inode *inode, struct nfs_fa
}
if ((fattr->valid & NFS_ATTR_FATTR_MTIME) != 0 &&
(fattr->valid & NFS_ATTR_FATTR_PREMTIME) == 0) {
- fattr->pre_mtime = inode->i_mtime;
+ fattr->pre_mtime = inode_get_mtime(inode);
fattr->valid |= NFS_ATTR_FATTR_PREMTIME;
}
if ((fattr->valid & NFS_ATTR_FATTR_SIZE) != 0 &&
@@ -2184,7 +2184,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
}
if (fattr->valid & NFS_ATTR_FATTR_MTIME)
- inode->i_mtime = fattr->mtime;
+ inode_set_mtime_to_ts(inode, fattr->mtime);
else if (fattr_supported & NFS_ATTR_FATTR_MTIME)
nfsi->cache_validity |=
save_cache_validity & NFS_INO_INVALID_MTIME;
@@ -2220,7 +2220,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
save_cache_validity & NFS_INO_INVALID_SIZE;
if (fattr->valid & NFS_ATTR_FATTR_ATIME)
- inode->i_atime = fattr->atime;
+ inode_set_atime_to_ts(inode, fattr->atime);
else if (fattr_supported & NFS_ATTR_FATTR_ATIME)
nfsi->cache_validity |=
save_cache_validity & NFS_INO_INVALID_ATIME;
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 01/87] fs: new accessor methods for atime and mtime
2023-09-28 11:02 [PATCH 00/87] fs: new accessor methods for atime and mtime Jeff Layton
@ 2023-09-28 11:03 ` Jeff Layton
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
1 sibling, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 11:03 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Linus Torvalds, David Sterba,
Amir Goldstein, Theodore Ts'o, Eric Biederman, Kees Cook,
Jeremy Kerr, Arnd Bergmann, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Carlos Llamas, Suren Baghdasaryan,
Mattia Dongili, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Hans de Goede,
Ilpo Järvinen, Mark Gross, Jiri Slaby, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian, Chris Mason,
Josef Bacik, Xiubo Li, Ilya Dryomov, Jan Harkes, coda,
Joel Becker, Christoph Hellwig, Nicolas Pitre, Rafael J. Wysocki,
Ard Biesheuvel, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Namjae Jeon, Sungjong Seo, Jan Kara, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Christoph Hellwig, Miklos Szeredi, Bob Peterson,
Andreas Gruenbacher, Richard Weinberger, Anton Ivanov,
Johannes Berg, Mikulas Patocka, Mike Kravetz, Muchun Song,
Jan Kara, David Woodhouse, Dave Kleikamp, Tejun Heo,
Trond Myklebust, Anna Schumaker, Chuck Lever, Neil Brown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Ryusuke Konishi,
Anton Altaparmakov, Konstantin Komarov, Mark Fasheh, Joseph Qi,
Bob Copeland, Mike Marshall, Martin Brandenburg, Luis Chamberlain,
Iurii Zaikin, Tony Luck, Guilherme G. Piccoli, Anders Larsen,
Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
Sergey Senozhatsky, Phillip Lougher, Steven Rostedt,
Masami Hiramatsu, Evgeniy Dushistov, Chandan Babu R,
Darrick J. Wong, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Hugh Dickins,
Andrew Morton, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Eric Paris
Cc: linux-fsdevel, linux-kernel, linux-mm, linuxppc-dev, linux-s390,
platform-driver-x86, linux-rdma, linux-serial, linux-usb, v9fs,
linux-afs, autofs, linux-btrfs, ceph-devel, codalist, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, gfs2, linux-um,
linux-mtd, jfs-discussion, linux-nfs, linux-nilfs, linux-ntfs-dev,
ntfs3, ocfs2-devel, linux-karma-devel, devel, linux-unionfs,
linux-hardening, reiserfs-devel, linux-cifs, samba-technical,
linux-trace-kernel, linux-xfs, bpf, netdev, apparmor,
linux-security-module, selinux
Recently, we converted the ctime accesses in the kernel to use new
accessor functions. Linus recently pointed out though that if we add
accessors for the atime and mtime, then that would allow us to
seamlessly change how these timestamps are stored in the inode.
Add new accessor functions for the atime and mtime that mirror the
accessors for the ctime.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/libfs.c | 13 +++++++++++++
include/linux/fs.h | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 37f2d34ee090..f5cdc7f7f5b5 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1912,3 +1912,16 @@ ssize_t direct_write_fallback(struct kiocb *iocb, struct iov_iter *iter,
return direct_written + buffered_written;
}
EXPORT_SYMBOL_GPL(direct_write_fallback);
+
+/**
+ * simple_inode_init_ts - initialize the timestamps for a new inode
+ * @inode: inode to be initialized
+ *
+ * When a new inode is created, most filesystems set the timestamps to the
+ * current time. Add a helper to do this.
+ */
+struct timespec64 simple_inode_init_ts(struct inode *inode);
+{
+ return inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
+}
+EXPORT_SYMBOL(simple_inode_init_ts);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b528f063e8ff..12d247b82aa0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1553,6 +1553,48 @@ static inline struct timespec64 inode_set_ctime(struct inode *inode,
return inode_set_ctime_to_ts(inode, ts);
}
+static inline struct timespec64 inode_get_atime(const struct inode *inode)
+{
+ return inode->i_atime;
+}
+
+static inline struct timespec64 inode_set_atime_to_ts(struct inode *inode,
+ struct timespec64 ts)
+{
+ inode->i_atime = ts;
+ return ts;
+}
+
+static inline struct timespec64 inode_set_atime(struct inode *inode,
+ time64_t sec, long nsec)
+{
+ struct timespec64 ts = { .tv_sec = sec,
+ .tv_nsec = nsec };
+ return inode_set_atime_to_ts(inode, ts);
+}
+
+static inline struct timespec64 inode_get_mtime(const struct inode *inode)
+{
+ return inode->i_mtime;
+}
+
+static inline struct timespec64 inode_set_mtime_to_ts(struct inode *inode,
+ struct timespec64 ts)
+{
+ inode->i_mtime = ts;
+ return ts;
+}
+
+static inline struct timespec64 inode_set_mtime(struct inode *inode,
+ time64_t sec, long nsec)
+{
+ struct timespec64 ts = { .tv_sec = sec,
+ .tv_nsec = nsec };
+ return inode_set_mtime_to_ts(inode, ts);
+}
+
+struct timespec64 simple_inode_init_ts(struct inode *inode);
+
/*
* Snapshotting support.
*/
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 51/87] fs/nfsd: convert to new inode {a,m}time accessors
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
2023-09-28 11:02 ` [PATCH 50/87] fs/nfs: convert to new inode {a,m}time accessors Jeff Layton
@ 2023-09-28 11:03 ` Jeff Layton
2023-09-28 13:56 ` Chuck Lever
2023-09-28 11:03 ` [PATCH 81/87] net/sunrpc: " Jeff Layton
2 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 11:03 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/blocklayout.c | 3 ++-
fs/nfsd/nfs3proc.c | 4 ++--
fs/nfsd/nfs4proc.c | 8 ++++----
fs/nfsd/nfsctl.c | 2 +-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
index 01d7fd108cf3..bdc582777738 100644
--- a/fs/nfsd/blocklayout.c
+++ b/fs/nfsd/blocklayout.c
@@ -119,10 +119,11 @@ nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
{
loff_t new_size = lcp->lc_last_wr + 1;
struct iattr iattr = { .ia_valid = 0 };
+ struct timespec64 mtime = inode_get_mtime(inode);
int error;
if (lcp->lc_mtime.tv_nsec == UTIME_NOW ||
- timespec64_compare(&lcp->lc_mtime, &inode->i_mtime) < 0)
+ timespec64_compare(&lcp->lc_mtime, &mtime) < 0)
lcp->lc_mtime = current_time(inode);
iattr.ia_valid |= ATTR_ATIME | ATTR_CTIME | ATTR_MTIME;
iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = lcp->lc_mtime;
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 268ef57751c4..b1c90a901d3e 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -294,8 +294,8 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
status = nfserr_exist;
break;
case NFS3_CREATE_EXCLUSIVE:
- if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
- d_inode(child)->i_atime.tv_sec == v_atime &&
+ if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
+ inode_get_atime(d_inode(child)).tv_sec == v_atime &&
d_inode(child)->i_size == 0) {
break;
}
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4199ede0583c..b17309aac0d5 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -322,8 +322,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
status = nfserr_exist;
break;
case NFS4_CREATE_EXCLUSIVE:
- if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
- d_inode(child)->i_atime.tv_sec == v_atime &&
+ if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
+ inode_get_atime(d_inode(child)).tv_sec == v_atime &&
d_inode(child)->i_size == 0) {
open->op_created = true;
break; /* subtle */
@@ -331,8 +331,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
status = nfserr_exist;
break;
case NFS4_CREATE_EXCLUSIVE4_1:
- if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
- d_inode(child)->i_atime.tv_sec == v_atime &&
+ if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
+ inode_get_atime(d_inode(child)).tv_sec == v_atime &&
d_inode(child)->i_size == 0) {
open->op_created = true;
goto set_attr; /* subtle */
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 7ed02fb88a36..846559e4769b 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1132,7 +1132,7 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
/* Following advice from simple_fill_super documentation: */
inode->i_ino = iunique(sb, NFSD_MaxReserved);
inode->i_mode = mode;
- inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
+ simple_inode_init_ts(inode);
switch (mode & S_IFMT) {
case S_IFDIR:
inode->i_fop = &simple_dir_operations;
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 81/87] net/sunrpc: convert to new inode {a,m}time accessors
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
2023-09-28 11:02 ` [PATCH 50/87] fs/nfs: convert to new inode {a,m}time accessors Jeff Layton
2023-09-28 11:03 ` [PATCH 51/87] fs/nfsd: " Jeff Layton
@ 2023-09-28 11:03 ` Jeff Layton
2 siblings, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 11:03 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs,
netdev
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
net/sunrpc/rpc_pipe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index f420d8457345..dcc2b4f49e77 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -472,7 +472,7 @@ rpc_get_inode(struct super_block *sb, umode_t mode)
return NULL;
inode->i_ino = get_next_ino();
inode->i_mode = mode;
- inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
+ simple_inode_init_ts(inode);
switch (mode & S_IFMT) {
case S_IFDIR:
inode->i_fop = &simple_dir_operations;
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 51/87] fs/nfsd: convert to new inode {a,m}time accessors
2023-09-28 11:03 ` [PATCH 51/87] fs/nfsd: " Jeff Layton
@ 2023-09-28 13:56 ` Chuck Lever
2023-09-28 14:09 ` Jeff Layton
0 siblings, 1 reply; 8+ messages in thread
From: Chuck Lever @ 2023-09-28 13:56 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel,
Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
On Thu, Sep 28, 2023 at 07:03:00AM -0400, Jeff Layton wrote:
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/blocklayout.c | 3 ++-
> fs/nfsd/nfs3proc.c | 4 ++--
> fs/nfsd/nfs4proc.c | 8 ++++----
> fs/nfsd/nfsctl.c | 2 +-
> 4 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index 01d7fd108cf3..bdc582777738 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -119,10 +119,11 @@ nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
> {
> loff_t new_size = lcp->lc_last_wr + 1;
> struct iattr iattr = { .ia_valid = 0 };
> + struct timespec64 mtime = inode_get_mtime(inode);
Nit: Please use reverse Christmas tree for new variable declarations.
> int error;
>
> if (lcp->lc_mtime.tv_nsec == UTIME_NOW ||
> - timespec64_compare(&lcp->lc_mtime, &inode->i_mtime) < 0)
> + timespec64_compare(&lcp->lc_mtime, &mtime) < 0)
> lcp->lc_mtime = current_time(inode);
> iattr.ia_valid |= ATTR_ATIME | ATTR_CTIME | ATTR_MTIME;
> iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = lcp->lc_mtime;
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 268ef57751c4..b1c90a901d3e 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -294,8 +294,8 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> status = nfserr_exist;
> break;
> case NFS3_CREATE_EXCLUSIVE:
> - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> - d_inode(child)->i_atime.tv_sec == v_atime &&
> + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
"inode_get_atime(yada).tv_sec" seems to be a frequently-repeated
idiom, at least in this patch. Would it be helpful to have an
additional helper that extracted just the seconds field, and one
that extracts just the nsec field?
> d_inode(child)->i_size == 0) {
> break;
> }
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 4199ede0583c..b17309aac0d5 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -322,8 +322,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> status = nfserr_exist;
> break;
> case NFS4_CREATE_EXCLUSIVE:
> - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> - d_inode(child)->i_atime.tv_sec == v_atime &&
> + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> d_inode(child)->i_size == 0) {
> open->op_created = true;
> break; /* subtle */
> @@ -331,8 +331,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> status = nfserr_exist;
> break;
> case NFS4_CREATE_EXCLUSIVE4_1:
> - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> - d_inode(child)->i_atime.tv_sec == v_atime &&
> + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> d_inode(child)->i_size == 0) {
> open->op_created = true;
> goto set_attr; /* subtle */
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 7ed02fb88a36..846559e4769b 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1132,7 +1132,7 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
> /* Following advice from simple_fill_super documentation: */
> inode->i_ino = iunique(sb, NFSD_MaxReserved);
> inode->i_mode = mode;
> - inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
> + simple_inode_init_ts(inode);
An observation about the whole series: Should these helpers use the
usual naming convention of:
<subsystem>-<subject>-<verb>
So we get:
simple_inode_ts_init(inode);
inode_atime_get(inode)
> switch (mode & S_IFMT) {
> case S_IFDIR:
> inode->i_fop = &simple_dir_operations;
> --
> 2.41.0
>
Otherwise, for the patch(es) touching nfsd:
Acked-by: Chuck Lever <chuck.lever@oracle.com>
--
Chuck Lever
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 51/87] fs/nfsd: convert to new inode {a,m}time accessors
2023-09-28 13:56 ` Chuck Lever
@ 2023-09-28 14:09 ` Jeff Layton
2023-09-28 14:33 ` Chuck Lever
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2023-09-28 14:09 UTC (permalink / raw)
To: Chuck Lever
Cc: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel,
Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
On Thu, 2023-09-28 at 09:56 -0400, Chuck Lever wrote:
> On Thu, Sep 28, 2023 at 07:03:00AM -0400, Jeff Layton wrote:
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > fs/nfsd/blocklayout.c | 3 ++-
> > fs/nfsd/nfs3proc.c | 4 ++--
> > fs/nfsd/nfs4proc.c | 8 ++++----
> > fs/nfsd/nfsctl.c | 2 +-
> > 4 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> > index 01d7fd108cf3..bdc582777738 100644
> > --- a/fs/nfsd/blocklayout.c
> > +++ b/fs/nfsd/blocklayout.c
> > @@ -119,10 +119,11 @@ nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
> > {
> > loff_t new_size = lcp->lc_last_wr + 1;
> > struct iattr iattr = { .ia_valid = 0 };
> > + struct timespec64 mtime = inode_get_mtime(inode);
>
> Nit: Please use reverse Christmas tree for new variable declarations.
>
Ok
>
> > int error;
> >
> > if (lcp->lc_mtime.tv_nsec == UTIME_NOW ||
> > - timespec64_compare(&lcp->lc_mtime, &inode->i_mtime) < 0)
> > + timespec64_compare(&lcp->lc_mtime, &mtime) < 0)
> > lcp->lc_mtime = current_time(inode);
> > iattr.ia_valid |= ATTR_ATIME | ATTR_CTIME | ATTR_MTIME;
> > iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = lcp->lc_mtime;
> > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> > index 268ef57751c4..b1c90a901d3e 100644
> > --- a/fs/nfsd/nfs3proc.c
> > +++ b/fs/nfsd/nfs3proc.c
> > @@ -294,8 +294,8 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > status = nfserr_exist;
> > break;
> > case NFS3_CREATE_EXCLUSIVE:
> > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
>
> "inode_get_atime(yada).tv_sec" seems to be a frequently-repeated
> idiom, at least in this patch. Would it be helpful to have an
> additional helper that extracted just the seconds field, and one
> that extracts just the nsec field?
>
I don't know that extra helpers will make that any clearer.
>
> > d_inode(child)->i_size == 0) {
> > break;
> > }
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 4199ede0583c..b17309aac0d5 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -322,8 +322,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > status = nfserr_exist;
> > break;
> > case NFS4_CREATE_EXCLUSIVE:
> > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> > d_inode(child)->i_size == 0) {
> > open->op_created = true;
> > break; /* subtle */
> > @@ -331,8 +331,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > status = nfserr_exist;
> > break;
> > case NFS4_CREATE_EXCLUSIVE4_1:
> > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> > d_inode(child)->i_size == 0) {
> > open->op_created = true;
> > goto set_attr; /* subtle */
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 7ed02fb88a36..846559e4769b 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1132,7 +1132,7 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
> > /* Following advice from simple_fill_super documentation: */
> > inode->i_ino = iunique(sb, NFSD_MaxReserved);
> > inode->i_mode = mode;
> > - inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
> > + simple_inode_init_ts(inode);
>
> An observation about the whole series: Should these helpers use the
> usual naming convention of:
>
> <subsystem>-<subject>-<verb>
>
> So we get:
>
> simple_inode_ts_init(inode);
>
> inode_atime_get(inode)
>
This was already bikeshedded during the ctime series, and the near
universal preference at the time was to go with inode_set_ctime and
inode_get_ctime. I'm just following suit with the new accessors.
>
> > switch (mode & S_IFMT) {
> > case S_IFDIR:
> > inode->i_fop = &simple_dir_operations;
> > --
> > 2.41.0
> >
>
> Otherwise, for the patch(es) touching nfsd:
>
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
>
Thanks!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 51/87] fs/nfsd: convert to new inode {a,m}time accessors
2023-09-28 14:09 ` Jeff Layton
@ 2023-09-28 14:33 ` Chuck Lever
0 siblings, 0 replies; 8+ messages in thread
From: Chuck Lever @ 2023-09-28 14:33 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, linux-fsdevel, linux-kernel,
Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
On Thu, Sep 28, 2023 at 10:09:19AM -0400, Jeff Layton wrote:
> On Thu, 2023-09-28 at 09:56 -0400, Chuck Lever wrote:
> > On Thu, Sep 28, 2023 at 07:03:00AM -0400, Jeff Layton wrote:
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > > fs/nfsd/blocklayout.c | 3 ++-
> > > fs/nfsd/nfs3proc.c | 4 ++--
> > > fs/nfsd/nfs4proc.c | 8 ++++----
> > > fs/nfsd/nfsctl.c | 2 +-
> > > 4 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> > > index 01d7fd108cf3..bdc582777738 100644
> > > --- a/fs/nfsd/blocklayout.c
> > > +++ b/fs/nfsd/blocklayout.c
> > > @@ -119,10 +119,11 @@ nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
> > > {
> > > loff_t new_size = lcp->lc_last_wr + 1;
> > > struct iattr iattr = { .ia_valid = 0 };
> > > + struct timespec64 mtime = inode_get_mtime(inode);
> >
> > Nit: Please use reverse Christmas tree for new variable declarations.
> >
>
> Ok
>
> >
> > > int error;
> > >
> > > if (lcp->lc_mtime.tv_nsec == UTIME_NOW ||
> > > - timespec64_compare(&lcp->lc_mtime, &inode->i_mtime) < 0)
> > > + timespec64_compare(&lcp->lc_mtime, &mtime) < 0)
> > > lcp->lc_mtime = current_time(inode);
> > > iattr.ia_valid |= ATTR_ATIME | ATTR_CTIME | ATTR_MTIME;
> > > iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = lcp->lc_mtime;
> > > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> > > index 268ef57751c4..b1c90a901d3e 100644
> > > --- a/fs/nfsd/nfs3proc.c
> > > +++ b/fs/nfsd/nfs3proc.c
> > > @@ -294,8 +294,8 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > > status = nfserr_exist;
> > > break;
> > > case NFS3_CREATE_EXCLUSIVE:
> > > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> >
> > "inode_get_atime(yada).tv_sec" seems to be a frequently-repeated
> > idiom, at least in this patch. Would it be helpful to have an
> > additional helper that extracted just the seconds field, and one
> > that extracts just the nsec field?
> >
>
> I don't know that extra helpers will make that any clearer.
To clarify my review comment:
I understand that eventually the timestamps stored in the inode will
be a single scalar value that is to be converted to a timespec64
structure.
So for accessors who want only one of the tv_sec or tv_nsec fields:
scalar -> timespec64 -> tv_sec
It might be more efficient to skip extracting the tv_nsec field when
that value isn't going to be used. Perhaps the compiler might
observe that the tv_nsec result isn't used and remove that dead
code. But IMO it would be easier for humans to understand and more
dependably optimized to write the helpers so the tv_nsec part of the
computation wasn't even in there in these cases.
> > > d_inode(child)->i_size == 0) {
> > > break;
> > > }
> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > index 4199ede0583c..b17309aac0d5 100644
> > > --- a/fs/nfsd/nfs4proc.c
> > > +++ b/fs/nfsd/nfs4proc.c
> > > @@ -322,8 +322,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > > status = nfserr_exist;
> > > break;
> > > case NFS4_CREATE_EXCLUSIVE:
> > > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> > > d_inode(child)->i_size == 0) {
> > > open->op_created = true;
> > > break; /* subtle */
> > > @@ -331,8 +331,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > > status = nfserr_exist;
> > > break;
> > > case NFS4_CREATE_EXCLUSIVE4_1:
> > > - if (d_inode(child)->i_mtime.tv_sec == v_mtime &&
> > > - d_inode(child)->i_atime.tv_sec == v_atime &&
> > > + if (inode_get_mtime(d_inode(child)).tv_sec == v_mtime &&
> > > + inode_get_atime(d_inode(child)).tv_sec == v_atime &&
> > > d_inode(child)->i_size == 0) {
> > > open->op_created = true;
> > > goto set_attr; /* subtle */
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 7ed02fb88a36..846559e4769b 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -1132,7 +1132,7 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
> > > /* Following advice from simple_fill_super documentation: */
> > > inode->i_ino = iunique(sb, NFSD_MaxReserved);
> > > inode->i_mode = mode;
> > > - inode->i_atime = inode->i_mtime = inode_set_ctime_current(inode);
> > > + simple_inode_init_ts(inode);
> >
> > An observation about the whole series: Should these helpers use the
> > usual naming convention of:
> >
> > <subsystem>-<subject>-<verb>
> >
> > So we get:
> >
> > simple_inode_ts_init(inode);
> >
> > inode_atime_get(inode)
> >
>
> This was already bikeshedded during the ctime series, and the near
> universal preference at the time was to go with inode_set_ctime and
> inode_get_ctime. I'm just following suit with the new accessors.
When this was reviewed before, there were only ctime accessors. Now
we have two more sets of accessor utilities. Just an observation. I
can drop it here.
> > > switch (mode & S_IFMT) {
> > > case S_IFDIR:
> > > inode->i_fop = &simple_dir_operations;
> > > --
> > > 2.41.0
> > >
> >
> > Otherwise, for the patch(es) touching nfsd:
> >
> > Acked-by: Chuck Lever <chuck.lever@oracle.com>
> >
>
> Thanks!
> --
> Jeff Layton <jlayton@kernel.org>
--
Chuck Lever
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-28 14:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-28 11:02 [PATCH 00/87] fs: new accessor methods for atime and mtime Jeff Layton
2023-09-28 11:03 ` [PATCH 01/87] " Jeff Layton
[not found] ` <20230928110413.33032-1-jlayton@kernel.org>
2023-09-28 11:02 ` [PATCH 50/87] fs/nfs: convert to new inode {a,m}time accessors Jeff Layton
2023-09-28 11:03 ` [PATCH 51/87] fs/nfsd: " Jeff Layton
2023-09-28 13:56 ` Chuck Lever
2023-09-28 14:09 ` Jeff Layton
2023-09-28 14:33 ` Chuck Lever
2023-09-28 11:03 ` [PATCH 81/87] net/sunrpc: " Jeff Layton
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).