* Re: [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [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; 19+ messages in thread
From: Jan Kara @ 2023-06-21 16:34 UTC (permalink / raw)
To: Jeff Layton
Cc: Latchesar Ionkov, Rafael J. Wysocki, Darrick J. Wong,
Anders Larsen, Carlos Llamas, Andrii Nakryiko, Hugh Dickins,
John Johansen, Seth Forshee, Alexander Gordeev, Christoph Hellwig,
Mike Marshall, Paulo Alcantara, linux-xfs, Bart Van Assche,
John Keeping, Zhang Yi, James Morris, Tyler Hicks, Alan Stern,
Christian Borntraeger, devel, Shyam Prasad N, Jan Harkes,
linux-um, Nicholas Piggin, Alexander Viro, Eric Van Hensbergen,
Suren Baghdasaryan, Trond Myklebust, Anton Altaparmakov,
Christian Brauner, Wolfram Sang, Greg Kroah-Hartman,
Stephen Smalley, linux-usb, linux-kernel, Ronnie Sahlberg,
Sergey Senozhatsky, Luis Chamberlain, Chuck Lever, Sven Schnelle,
Jiri Olsa, Jan Kara, Tejun Heo, Andrew Morton, linux-trace-kernel,
linux-hardening, Dave Kleikamp, Sandeep Dhavale, Tetsuo Handa,
Mimi Zohar, linux-mm, Joel Fernandes, Eric Dumazet,
Stanislav Fomichev, Andrzej Pietrasiewicz, Hangyu Hua, linux-s390,
linux-nilfs, Paul Moore, Leon Romanovsky, John Fastabend,
Arve Hjønnevåg, Minghao Chi, codalist, selinux,
ZhangPeng, Udipto Goswami, Yonghong Song, Iurii Zaikin,
Namjae Jeon, Masami Hiramatsu, ecryptfs, Todd Kjos, Vasily Gorbik,
Yu Zhe, linuxppc-dev, reiserfs-devel, Miklos Szeredi, Yue Hu,
Jaegeuk Kim, Aditya Garg, Martijn Coenen, OGAWA Hirofumi, Hao Luo,
Tony Luck, Theodore Ts'o, Nicolas Pitre, linux-ntfs-dev,
Muchun Song, Roberto Sassu, linux-f2fs-devel,
Guilherme G. Piccoli, Jozef Martiniak, Eric Biederman,
Anna Schumaker, xu xin, Brad Warrum, Mike Kravetz, Jingyu Wang,
linux-efi, Dan Carpenter, Martin Brandenburg, Tom Rix,
Alexei Starovoitov, Chris Mason, linux-mtd,
Matthew Wilcox (Oracle), Marc Dionne, linux-afs, Ian Kent,
Naohiro Aota, Daniel Borkmann, Dennis Dalessandro, linux-rdma,
Linyu Yuan, coda, Viacheslav Dubeyko, Ilya Dryomov, Paolo Abeni,
Alexey Dobriyan, Serge E. Hallyn, Zhihao Cheng, Jens Axboe,
Zeng Jingxiang, Kees Cook, Arnd Bergmann, autofs, Steven Rostedt,
Yifei Liu, Damien Le Moal, Eric Paris, ceph-devel, Gao Xiang,
Jiangshan Yi, David Howells, linux-nfs, linux-ext4, Song Liu,
samba-technical, Steve French, Jeremy Kerr, netdev, Bob Peterson,
linux-fsdevel, bpf, ntfs3, linux-erofs, David S. Miller,
ocfs2-devel, jfs-discussion, Dominique Martinet,
Christian Schoenebeck, Bob Copeland, KP Singh, Oleg Kanatov,
Konstantin Komarov, Joseph Qi, Yuta Hayama, Andreas Dilger,
Mikulas Patocka, Zhengchao Shao, Chen Zhongjin, Ard Biesheuvel,
Anton Ivanov, Laurent Pinchart, Andreas Gruenbacher
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] 19+ messages in thread* Re: [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [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; 19+ messages in thread
From: Tom Talpey @ 2023-06-21 17:29 UTC (permalink / raw)
To: Jeff Layton, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Hans de Goede,
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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread* Re: [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; 19+ messages in thread
From: Jeff Layton @ 2023-06-21 18:01 UTC (permalink / raw)
To: Tom Talpey, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Hans de Goede,
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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread* Re: [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; 19+ messages in thread
From: Tom Talpey @ 2023-06-21 18:19 UTC (permalink / raw)
To: Jeff Layton, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Hans de Goede,
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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread* Re: [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; 19+ messages in thread
From: Jeff Layton @ 2023-06-21 18:48 UTC (permalink / raw)
To: Tom Talpey, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Hans de Goede,
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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread
* Re: [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [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; 19+ messages in thread
From: Damien Le Moal @ 2023-06-22 0:46 UTC (permalink / raw)
To: Jeff Layton, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Tom Talpey,
Sergey Senozhatsky, Phillip Lougher, Steven Rostedt,
Masami Hiramatsu, Evgeniy Dushistov, Hans de Goede,
Darrick J. Wong, 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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread
* Re: [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; 19+ messages in thread
From: Jeff Layton @ 2023-06-22 10:14 UTC (permalink / raw)
To: Damien Le Moal, 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, Christian Brauner, Carlos Llamas,
Suren Baghdasaryan, Dennis Dalessandro, Jason Gunthorpe,
Leon Romanovsky, Brad Warrum, Ritu Agarwal, Eric Van Hensbergen,
Latchesar Ionkov, Dominique Martinet, Christian Schoenebeck,
David Sterba, David Howells, Marc Dionne, Alexander Viro,
Ian Kent, Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Eric Biederman, Kees Cook, Chris Mason, Josef Bacik, Xiubo Li,
Ilya Dryomov, Jan Harkes, coda, Joel Becker, Christoph Hellwig,
Nicolas Pitre, Rafael J. Wysocki, Tyler Hicks, Ard Biesheuvel,
Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Namjae Jeon, Sungjong Seo,
Jan Kara, Theodore Ts'o, Andreas Dilger, Jaegeuk Kim,
OGAWA Hirofumi, Miklos Szeredi, Bob Peterson, Andreas Gruenbacher,
Richard Weinberger, Anton Ivanov, Johannes Berg, Mikulas Patocka,
Mike Kravetz, Muchun Song, David Woodhouse, Dave Kleikamp,
Tejun Heo, Trond Myklebust, Anna Schumaker, Chuck Lever,
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, Tom Talpey,
Sergey Senozhatsky, Phillip Lougher, Steven Rostedt,
Masami Hiramatsu, Evgeniy Dushistov, Hans de Goede,
Darrick J. Wong, 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, Juergen Gross,
Ruihan Li, Laurent Pinchart, Wolfram Sang, Udipto Goswami,
Linyu Yuan, John Keeping, Andrzej Pietrasiewicz, Dan Carpenter,
Yuta Hayama, Jozef Martiniak, Jens Axboe, Alan Stern,
Sandeep Dhavale, Dave Chinner, Johannes Weiner, ZhangPeng,
Viacheslav Dubeyko, Tetsuo Handa, Aditya Garg, Erez Zadok,
Yifei Liu, Yu Zhe, Matthew Wilcox (Oracle), Oleg Kanatov,
Dr. David Alan Gilbert, Jiangshan Yi, xu xin, Stefan Roesch,
Zhihao Cheng, Liam R. Howlett, Alexey Dobriyan, Minghao Chi,
Seth Forshee, Zeng Jingxiang, Bart Van Assche, Mimi Zohar,
Roberto Sassu, Zhang Yi, Tom Rix, Fabio M. De Francesco,
Chen Zhongjin, Zhengchao Shao, Rik van Riel, Jingyu Wang,
Hangyu Hua, linuxppc-dev, linux-kernel, linux-s390, linux-rdma,
linux-usb, v9fs, linux-fsdevel, linux-afs, autofs, linux-mm,
linux-btrfs, ceph-devel, codalist, ecryptfs, linux-efi,
linux-erofs, linux-ext4, linux-f2fs-devel, cluster-devel,
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
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] 19+ messages in thread
* Re: [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [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; 19+ messages in thread
From: Luis Chamberlain @ 2023-06-30 22:12 UTC (permalink / raw)
To: Jeff Layton
Cc: Latchesar Ionkov, Rafael J. Wysocki, Darrick J. Wong,
Anders Larsen, Carlos Llamas, Hugh Dickins, John Johansen,
Seth Forshee, Alexander Gordeev, Christoph Hellwig, Mike Marshall,
Paulo Alcantara, linux-xfs, Bart Van Assche, John Keeping,
Zhang Yi, James Morris, Tyler Hicks, Alan Stern,
Christian Borntraeger, devel, Shyam Prasad N, Jan Harkes,
linux-um, Nicholas Piggin, Alexander Viro, Eric Van Hensbergen,
Suren Baghdasaryan, Trond Myklebust, Anton Altaparmakov,
Christian Brauner, Wolfram Sang, Greg Kroah-Hartman,
Stephen Smalley, linux-usb, linux-kernel, Ronnie Sahlberg,
Sergey Senozhatsky, Arve Hjønnevåg, Chuck Lever,
Sven Schnelle, Jiri Olsa, Jan Kara, Tejun Heo, Andrew Morton,
linux-trace-kernel, linux-hardening, Dave Kleikamp,
Sandeep Dhavale, Tetsuo Handa, Mimi Zohar, linux-mm,
Joel Fernandes
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] 19+ messages in thread* Re: [PATCH 01/79] fs: add ctime accessors infrastructure
2023-06-21 14:45 ` [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; 19+ messages in thread
From: Randy Dunlap @ 2023-07-12 15:31 UTC (permalink / raw)
To: Jeff Layton, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, linux-um
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] 19+ messages in thread