Message-ID: <e513d856-3a6f-3a32-40fe-6c728e7b5ec8@talpey.com>
Date: Wed, 21 Jun 2023 14:19:56 -0400
From: Tom Talpey <tom@...pey.com>
To: Jeff Layton <jlayton@...nel.org>, Jeremy Kerr <jk@...abs.org>,
 Arnd Bergmann <arnd@...db.de>, Michael Ellerman <mpe@...erman.id.au>,
 Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
 Alexander Gordeev <agordeev@...ux.ibm.com>,
 Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Sven Schnelle <svens@...ux.ibm.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Arve Hjønnevåg <arve@...roid.com>,
 Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>,
 Joel Fernandes <joel@...lfernandes.org>,
 Christian Brauner <brauner@...nel.org>, Carlos Llamas <cmllamas@...gle.com>,
 Suren Baghdasaryan <surenb@...gle.com>,
 Dennis Dalessandro <dennis.dalessandro@...nelisnetworks.com>,
 Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
 Brad Warrum <bwarrum@...ux.ibm.com>, Ritu Agarwal <rituagar@...ux.ibm.com>,
 Eric Van Hensbergen <ericvh@...nel.org>, Latchesar Ionkov
 <lucho@...kov.net>, Dominique Martinet <asmadeus@...ewreck.org>,
 Christian Schoenebeck <linux_oss@...debyte.com>,
 David Sterba <dsterba@...e.com>, David Howells <dhowells@...hat.com>,
 Marc Dionne <marc.dionne@...istor.com>,
 Alexander Viro <viro@...iv.linux.org.uk>, Ian Kent <raven@...maw.net>,
 Luis de Bethencourt <luisbg@...nel.org>, Salah Triki
 <salah.triki@...il.com>, "Tigran A. Aivazian" <aivazian.tigran@...il.com>,
 Eric Biederman <ebiederm@...ssion.com>, Kees Cook <keescook@...omium.org>,
 Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
 Xiubo Li <xiubli@...hat.com>, Ilya Dryomov <idryomov@...il.com>,
 Jan Harkes <jaharkes@...cmu.edu>, coda@...cmu.edu,
 Joel Becker <jlbec@...lplan.org>, Christoph Hellwig <hch@....de>,
 Nicolas Pitre <nico@...xnic.net>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Tyler Hicks <code@...icks.com>, Ard Biesheuvel <ardb@...nel.org>,
 Gao Xiang <xiang@...nel.org>, Chao Yu <chao@...nel.org>,
 Yue Hu <huyue2@...lpad.com>, Jeffle Xu <jefflexu@...ux.alibaba.com>,
 Namjae Jeon <linkinjeon@...nel.org>, Sungjong Seo <sj1557.seo@...sung.com>,
 Jan Kara <jack@...e.com>, Theodore Ts'o <tytso@....edu>,
 Andreas Dilger <adilger.kernel@...ger.ca>, Jaegeuk Kim <jaegeuk@...nel.org>,
 OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
 Miklos Szeredi <miklos@...redi.hu>, Bob Peterson <rpeterso@...hat.com>,
 Andreas Gruenbacher <agruenba@...hat.com>,
 Richard Weinberger <richard@....at>,
 Anton Ivanov <anton.ivanov@...bridgegreys.com>,
 Johannes Berg <johannes@...solutions.net>,
 Mikulas Patocka <mikulas@...ax.karlin.mff.cuni.cz>,
 Mike Kravetz <mike.kravetz@...cle.com>, Muchun Song <muchun.song@...ux.dev>,
 David Woodhouse <dwmw2@...radead.org>, Dave Kleikamp <shaggy@...nel.org>,
 Tejun Heo <tj@...nel.org>, Trond Myklebust
 <trond.myklebust@...merspace.com>, Anna Schumaker <anna@...nel.org>,
 Chuck Lever <chuck.lever@...cle.com>,
 Ryusuke Konishi <konishi.ryusuke@...il.com>,
 Anton Altaparmakov <anton@...era.com>,
 Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
 Mark Fasheh <mark@...heh.com>, Joseph Qi <joseph.qi@...ux.alibaba.com>,
 Bob Copeland <me@...copeland.com>, Mike Marshall <hubcap@...ibond.com>,
 Martin Brandenburg <martin@...ibond.com>,
 Luis Chamberlain <mcgrof@...nel.org>, Iurii Zaikin <yzaikin@...gle.com>,
 Tony Luck <tony.luck@...el.com>, "Guilherme G. Piccoli"
 <gpiccoli@...lia.com>, Anders Larsen <al@...rsen.net>,
 Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>,
 Ronnie Sahlberg <lsahlber@...hat.com>, Shyam Prasad N
 <sprasad@...rosoft.com>, Sergey Senozhatsky <senozhatsky@...omium.org>,
 Phillip Lougher <phillip@...ashfs.org.uk>,
 Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Evgeniy Dushistov <dushistov@...l.ru>,
 Hans de Goede <hdegoede@...hat.com>, "Darrick J. Wong" <djwong@...nel.org>,
 Damien Le Moal <dlemoal@...nel.org>, Naohiro Aota <naohiro.aota@....com>,
 Johannes Thumshirn <jth@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
 Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Hugh Dickins <hughd@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 John Johansen <john.johansen@...onical.com>, Paul Moore
 <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
 "Serge E. Hallyn" <serge@...lyn.com>,
 Stephen Smalley <stephen.smalley.work@...il.com>,
 Eric Paris <eparis@...isplace.org>, Juergen Gross <jgross@...e.com>,
 Ruihan Li <lrh2000@....edu.cn>,
 Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Udipto Goswami <quic_ugoswami@...cinc.com>,
 Linyu Yuan <quic_linyyuan@...cinc.com>, John Keeping <john@...ping.me.uk>,
 Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
 Dan Carpenter <error27@...il.com>, Yuta Hayama <hayama@...eo.co.jp>,
 Jozef Martiniak <jomajm@...il.com>, Jens Axboe <axboe@...nel.dk>,
 Alan Stern <stern@...land.harvard.edu>, Sandeep Dhavale
 <dhavale@...gle.com>, Dave Chinner <dchinner@...hat.com>,
 Johannes Weiner <hannes@...xchg.org>, ZhangPeng <zhangpeng362@...wei.com>,
 Viacheslav Dubeyko <slava@...eyko.com>,
 Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
 Aditya Garg <gargaditya08@...e.com>, Erez Zadok <ezk@...stonybrook.edu>,
 Yifei Liu <yifeliu@...stonybrook.edu>, Yu Zhe <yuzhe@...china.com>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Oleg Kanatov <okanatov@...il.com>, "Dr. David Alan Gilbert"
 <linux@...blig.org>, Jiangshan Yi <yijiangshan@...inos.cn>,
 xu xin <cgel.zte@...il.com>, Stefan Roesch <shr@...kernel.io>,
 Zhihao Cheng <chengzhihao1@...wei.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Alexey Dobriyan <adobriyan@...il.com>, Minghao Chi <chi.minghao@....com.cn>,
 Seth Forshee <sforshee@...italocean.com>,
 Zeng Jingxiang <linuszeng@...cent.com>, Bart Van Assche
 <bvanassche@....org>, Mimi Zohar <zohar@...ux.ibm.com>,
 Roberto Sassu <roberto.sassu@...wei.com>, Zhang Yi <yi.zhang@...wei.com>,
 Tom Rix <trix@...hat.com>, "Fabio M. De Francesco"
 <fmdefrancesco@...il.com>, Chen Zhongjin <chenzhongjin@...wei.com>,
 Zhengchao Shao <shaozhengchao@...wei.com>, Rik van Riel <riel@...riel.com>,
 Jingyu Wang <jingyuwang_vip@....com>, Hangyu Hua <hbh25y@...il.com>,
 linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
 linux-s390@...r.kernel.org, linux-rdma@...r.kernel.org,
 linux-usb@...r.kernel.org, v9fs@...ts.linux.dev,
 linux-fsdevel@...r.kernel.org, linux-afs@...ts.infradead.org,
 autofs@...r.kernel.org, linux-mm@...ck.org, linux-btrfs@...r.kernel.org,
 ceph-devel@...r.kernel.org, codalist@...a.cs.cmu.edu,
 ecryptfs@...r.kernel.org, linux-efi@...r.kernel.org,
 linux-erofs@...ts.ozlabs.org, linux-ext4@...r.kernel.org,
 linux-f2fs-devel@...ts.sourceforge.net, cluster-devel@...hat.com,
 linux-um@...ts.infradead.org, linux-mtd@...ts.infradead.org,
 jfs-discussion@...ts.sourceforge.net, linux-nfs@...r.kernel.org,
 linux-nilfs@...r.kernel.org, linux-ntfs-dev@...ts.sourceforge.net,
 ntfs3@...ts.linux.dev, ocfs2-devel@....oracle.com,
 linux-karma-devel@...ts.sourceforge.net, devel@...ts.orangefs.org,
 linux-unionfs@...r.kernel.org, linux-hardening@...r.kernel.org,
 reiserfs-devel@...r.kernel.org, linux-cifs@...r.kernel.org,
 samba-technical@...ts.samba.org, linux-trace-kernel@...r.kernel.org,
 linux-xfs@...r.kernel.org, bpf@...r.kernel.org, netdev@...r.kernel.org,
 apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org,
 selinux@...r.kernel.org
Subject: Re: [PATCH 01/79] fs: add ctime accessors infrastructure

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@...nel.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@...pey.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.
> 

Powered by blists - more mailing lists