From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 835D0C77B6F for ; Tue, 11 Apr 2023 14:54:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E47736B0072; Tue, 11 Apr 2023 10:54:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DD1316B0075; Tue, 11 Apr 2023 10:54:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C984C900002; Tue, 11 Apr 2023 10:54:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id B70C46B0072 for ; Tue, 11 Apr 2023 10:54:35 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 6AAEBA0BA8 for ; Tue, 11 Apr 2023 14:54:35 +0000 (UTC) X-FDA: 80669406510.29.10E05AC Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf03.hostedemail.com (Postfix) with ESMTP id 99BA120023 for ; Tue, 11 Apr 2023 14:54:32 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=gKk6NqKl; spf=pass (imf03.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681224872; a=rsa-sha256; cv=none; b=VeweI5xB//2GpzzDqfKRdvtNiSPg76iMuwqPv4OPJw+6aQszU0rzm/W+8yY8ORFgz/ve8L XWtaxiW68zWSo1/tIJglX/Anl0t3X5NV+aJxrNPEv+UFoWEDzw4DZS55k8QcdeDnX8X0L/ nGHlHPr1BvEF7emH85kBeJajAjcjCyQ= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=gKk6NqKl; spf=pass (imf03.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681224872; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=0E2IKBlfidQt38RWQiCXlV93eIpozuAr2210KvR/OAM=; b=BBCDlr3ZE7LmAr//LcNa5s0AXtuOCvl/8HZ3EAQUHZbidahQFTrLyAZCSRLjndbijRJKdw wkK6pjr/r0EmeXGcetuLjAyM7mLpXMIQbgv29ZhCa4I/tjLGVQR2AphSeUhX7AqW07kFeB REk3tz5+EdBDfbkZQenhgVEpUQo3jqk= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 92EE562800; Tue, 11 Apr 2023 14:54:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CEF39C433D2; Tue, 11 Apr 2023 14:54:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681224871; bh=yabeQv2YXsln9+A4U/WF7gDOBnNuivtv0ge605jAWvA=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=gKk6NqKlLmnyt+TruttglVTb3e8/mVYyKO91oqeIk9GpnY71WqWBSM5UOlecjQT9b ZtJhLFL3oo0iTWpzmnJ9F9kr2+q08aDcTQiXG2kaRtq94Y8qMQ2ASxK4TnuYjMt5+p zjQlOIP3DEXbU/ZvK2WYZBF3XLmKr3MX1j6dMJJbR641nNeQ3bYTdLXRhJ0PNaGBFr 7VtKMzCy2nIz5Ln1cKWMYlb2TrORHrMRsWmVp063ZKdZMF7MpYr5oy6Kky+0AqjYEC OYJ236mqEFOcTUHm6lioE1SRs4UbG3hrDeGjO1jO8PkYdaXQrtCphYqXqsXWjnOFK3 dSo89U3O3SLPw== Message-ID: <99ed0a98587ab762dd5cdc1e81e4224936aa4808.camel@kernel.org> Subject: Re: [RFC PATCH 1/3][RESEND] fs: add infrastructure for opportunistic high-res ctime/mtime updates From: Jeff Layton To: "Darrick J. Wong" Cc: Alexander Viro , Christian Brauner , Hugh Dickins , Andrew Morton , Dave Chinner , Chuck Lever , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-nfs@vger.kernel.org Date: Tue, 11 Apr 2023 10:54:28 -0400 In-Reply-To: <20230411144232.GF360895@frogsfrogsfrogs> References: <20230411143702.64495-1-jlayton@kernel.org> <20230411143702.64495-2-jlayton@kernel.org> <20230411144232.GF360895@frogsfrogsfrogs> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) MIME-Version: 1.0 X-Rspam-User: X-Rspamd-Queue-Id: 99BA120023 X-Rspamd-Server: rspam01 X-Stat-Signature: jmcioib5ti7a5ckerd8qtos34aacyhgz X-HE-Tag: 1681224872-564119 X-HE-Meta: U2FsdGVkX18+iXI/TKUlps56V9sAJZg71X24WPodhsNnSAjxOiQECVC6mgnrUZsEuON2XaOJ7SpXazVRprreuvL6lJ9psGUR7Pfkyshn9/tiG6jq9BTtLst/VNALbvODA8V181QJnksvNSLHWIFBLiXRwGOP++qAXQngOua2sOY0u56yDSNtHBL/7N1d5h0x0cZ9de3z8rhBS/tKhJJvs8x7wfNmvykbnNABj/jqTD9XkXFbilkNjHDyE3IeE/arqGcSrNmIpPr7P5775JxfyiizJNXeV4yOGs5V7saMVHK+zwMdS0LT0QGRyXxPQESOKtSpRX4yt3uTJ9om8Kg5r4rLDCH+FKTb80vzlNGQIlF+2/iRNLURz+cKepDWlOWd4qNc7dHt88d2j0Xpvs5oZTyaszpBmKreI/P6nwvyRoKeQEKuk4H2HF7c7eaBfiI8YBmav02dJmlR5yK9jsvJTI3nA3v2i2yGlhUS5waiZYToNwjH18vozR+TqE1BAyE7VfSpNGgRz7XubTsiqBvyPhT2bu1Bggx2CcE+r+BYZ5ssqRJDG0tIy1sQ+EHpicff2a2Fs8t0koe/EGDsjNs6aXyrlzAF/Bmgy4AMPHxbGZ4zN+MZMjcrI7DpGFIZbLK2PrNU0L/xq9QSEf2BfIKI1B1nea7wCCK0MGmFwpNoyUrtOU2DgW3+HiUSPIk1tIymtNG2j68Hj2x+NnWR4Jx9O5xSfW7WAqpi/5ysdTc1UiRIZJbAQBABCHA1voKR4puzrQKFZehmguAaeEzTh43VYruvT/jQERt13ONcnZb8E/O1x4wPVmrMUCbJ0+8YQ2zn6V3HtGe+qBQe5tLTdcrHhhOe5bugkyT3rJ02TXwhqAnHEZFUQyi5H03BVGiDFB+HeY0LzlCckG4DKMouXZHX7JDT/bLoeObGp2rmzQ0+kI5aP0WPtkLtxBpzX/YOgoEa8QLJfh/BnPBByMZmKMH pCw2Vz9a O3+0ld0w6YSECWKOJRgqyCYdHpiG5Fh4K5R0+cl/jJDnqDTQMADHEqyZ56S4cCF/yLfRclcsqe1fuH//b8I2vVkEbISWjd7vfXTlknCBGw3MuHh3z7zMehfgRdt8vIly6JEtxOBdM9MpZnUrYl2mzKQBj8HlkI2oEZNuoeHHCHxRm/97ETPcwGMi/0UpCPGanhfA+O9opdYGRkzUXCz/iXjtChRCo41mqWyOidg1wYG5er2wvXysrd3Iy0Fg2jJt0uJK8GXpJPNgqc1GfZ2BgLcgRII63W4zwOeIiGHKjJi5rF3Qcb6voyyfsz35hO5LBU9kAx8ORslj1WU6A5dfM13DVm8ZW2PheIfIz8PClf1aqz2ih1KFQgycPRc03OiQhJtR6XVRg1qY2/Y4pV/igliWGJE6g+o5u6OvBTkrlVAnh8OE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 2023-04-11 at 07:42 -0700, Darrick J. Wong wrote: > On Tue, Apr 11, 2023 at 10:37:00AM -0400, Jeff Layton wrote: > > The VFS always uses coarse-grained timestamp updates for filling out th= e > > ctime and mtime after a change. This has the benefit of allowing > > filesystems to optimize away metadata updates. > >=20 > > Unfortunately, this has always been an issue when we're exporting via > > NFSv3, which relies on timestamps to validate caches. Even with NFSv4, = a > > lot of exported filesystems don't properly support a change attribute > > and are subject to the same problem of timestamp granularity. Other > > applications have similar issues (e.g backup applications). > >=20 > > Switching to always using high resolution timestamps would improve the > > situation for NFS, but that becomes rather expensive, as we'd have to > > log a lot more metadata updates. > >=20 > > This patch grabs a new i_state bit to use as a flag that filesystems ca= n > > set in their getattr routine to indicate that the mtime or ctime was > > queried since it was last updated. > >=20 > > It then adds a new current_cmtime function that acts like the > > current_time helper, but will conditionally grab high-res timestamps > > when the i_state flag is set in the inode. > >=20 > > This allows NFS and other applications to reap the benefits of high-res > > ctime and mtime timestamps, but at a substantially lower cost than > > fetching them every time. > >=20 > > Cc: Dave Chinner > > Signed-off-by: Jeff Layton > > --- > > fs/inode.c | 40 ++++++++++++++++++++++++++++++++++++++-- > > fs/stat.c | 10 ++++++++++ > > include/linux/fs.h | 5 ++++- > > 3 files changed, 52 insertions(+), 3 deletions(-) > >=20 > > diff --git a/fs/inode.c b/fs/inode.c > > index 4558dc2f1355..3630f67fd042 100644 > > --- a/fs/inode.c > > +++ b/fs/inode.c > > @@ -2062,6 +2062,42 @@ static int __file_update_time(struct file *file,= struct timespec64 *now, > > return ret; > > } > > =20 > > +/** > > + * current_cmtime - Return FS time (possibly high-res) > > + * @inode: inode. > > + * > > + * Return the current time truncated to the time granularity supported= by > > + * the fs, as suitable for a ctime or mtime change. If something recen= tly > > + * fetched the ctime or mtime out of the inode via getattr, then get a > > + * high-resolution timestamp. > > + * > > + * Note that inode and inode->sb cannot be NULL. > > + * Otherwise, the function warns and returns coarse time without trunc= ation. > > + */ > > +struct timespec64 current_cmtime(struct inode *inode) > > +{ > > + struct timespec64 now; > > + > > + if (unlikely(!inode->i_sb)) { > > + WARN(1, "%s() called with uninitialized super_block in the inode", _= _func__); > > + ktime_get_coarse_real_ts64(&now); > > + return now; > > + } > > + > > + /* Do a lockless check for the flag before taking the spinlock */ > > + if (READ_ONCE(inode->i_state) & I_CMTIME_QUERIED) { > > + ktime_get_real_ts64(&now); > > + spin_lock(&inode->i_lock); > > + inode->i_state &=3D ~I_CMTIME_QUERIED; > > + spin_unlock(&inode->i_lock); > > + } else { > > + ktime_get_coarse_real_ts64(&now); > > + } > > + > > + return timestamp_truncate(now, inode); >=20 > I wonder, under which conditions (arch+fs) would it be worth the effort > to check s_time_gran as part of deciding whether or not to sample a high > res timestamp? >=20 > I suppose that would only help us for the situation where "ktime > sampling is not fast" and "fs timestamp granularity is awful"? >=20 > (Mechanically, the function body looks ok to me...) >=20 Thanks for looking! Yeah, that is a good point. No reason to fetch a high res value if we can't store it anyway. That shouldn't be hard to optimize away. I'll plan to have a look at that. > > +} > > +EXPORT_SYMBOL(current_cmtime); > > + > > /** > > * file_update_time - update mtime and ctime time > > * @file: file accessed > > @@ -2080,7 +2116,7 @@ int file_update_time(struct file *file) > > { > > int ret; > > struct inode *inode =3D file_inode(file); > > - struct timespec64 now =3D current_time(inode); > > + struct timespec64 now =3D current_cmtime(inode); > > =20 > > ret =3D inode_needs_update_time(inode, &now); > > if (ret <=3D 0) > > @@ -2109,7 +2145,7 @@ static int file_modified_flags(struct file *file,= int flags) > > { > > int ret; > > struct inode *inode =3D file_inode(file); > > - struct timespec64 now =3D current_time(inode); > > + struct timespec64 now =3D current_cmtime(inode); > > =20 > > /* > > * Clear the security bits if the process is not being run by root. > > diff --git a/fs/stat.c b/fs/stat.c > > index 7c238da22ef0..d8b80a2e36b7 100644 > > --- a/fs/stat.c > > +++ b/fs/stat.c > > @@ -64,6 +64,16 @@ void generic_fillattr(struct mnt_idmap *idmap, struc= t inode *inode, > > } > > EXPORT_SYMBOL(generic_fillattr); > > =20 > > +void fill_cmtime_and_mark(struct inode *inode, struct kstat *stat) > > +{ > > + spin_lock(&inode->i_lock); > > + inode->i_state |=3D I_CMTIME_QUERIED; > > + stat->ctime =3D inode->i_ctime; > > + stat->mtime =3D inode->i_mtime; > > + spin_unlock(&inode->i_lock); > > +} > > +EXPORT_SYMBOL(fill_cmtime_and_mark); > > + > > /** > > * generic_fill_statx_attr - Fill in the statx attributes from the ino= de flags > > * @inode: Inode to use as the source > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index c85916e9f7db..7dece4390979 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -1457,7 +1457,8 @@ static inline bool fsuidgid_has_mapping(struct su= per_block *sb, > > kgid_has_mapping(fs_userns, kgid); > > } > > =20 > > -extern struct timespec64 current_time(struct inode *inode); > > +struct timespec64 current_time(struct inode *inode); > > +struct timespec64 current_cmtime(struct inode *inode); > > =20 > > /* > > * Snapshotting support. > > @@ -2116,6 +2117,7 @@ static inline void kiocb_clone(struct kiocb *kioc= b, struct kiocb *kiocb_src, > > #define I_DONTCACHE (1 << 16) > > #define I_SYNC_QUEUED (1 << 17) > > #define I_PINNING_FSCACHE_WB (1 << 18) > > +#define I_CMTIME_QUERIED (1 << 19) > > =20 > > #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC) > > #define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES) > > @@ -2839,6 +2841,7 @@ extern int page_symlink(struct inode *inode, cons= t char *symname, int len); > > extern const struct inode_operations page_symlink_inode_operations; > > extern void kfree_link(void *); > > void generic_fillattr(struct mnt_idmap *, struct inode *, struct kstat= *); > > +void fill_cmtime_and_mark(struct inode *inode, struct kstat *stat); > > void generic_fill_statx_attr(struct inode *inode, struct kstat *stat); > > extern int vfs_getattr_nosec(const struct path *, struct kstat *, u32,= unsigned int); > > extern int vfs_getattr(const struct path *, struct kstat *, u32, unsig= ned int); > > --=20 > > 2.39.2 > >=20 --=20 Jeff Layton