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 56DF2C77B61 for ; Thu, 27 Apr 2023 09:45:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A6E2F900002; Thu, 27 Apr 2023 05:45:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A1EB46B0075; Thu, 27 Apr 2023 05:45:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8E62D900002; Thu, 27 Apr 2023 05:45:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 7ED386B0074 for ; Thu, 27 Apr 2023 05:45:07 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 411D2140246 for ; Thu, 27 Apr 2023 09:45:07 +0000 (UTC) X-FDA: 80726687454.22.092A460 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf03.hostedemail.com (Postfix) with ESMTP id 8561020020 for ; Thu, 27 Apr 2023 09:45:04 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ky7i7k6Q; spf=pass (imf03.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682588704; a=rsa-sha256; cv=none; b=e0OJwwDV7p+/wf6REW0exN7fgMHp2JSs887EFtO9+OWoD/0J6dC94erVRBt5/nbwez4q71 cFuwrFbMgYiJGWKBlwyAsxUURsej9bHjOfBUTJBKzBe7owUDaWfq1Sf74zDSa50gObHcB+ 4BDSdr6pGV6QXrUYqg3M2s3oslnPKrw= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ky7i7k6Q; spf=pass (imf03.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@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=1682588704; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dXC6sABmwLOEXdEP3EmfXrRrX9g65m2XQLCyzc+0C5o=; b=k6FDp9FetQlxGVX4nDdQtF4o+VsoW5tKKFh+h55r/sHr6in8Le5hNTY/YR/GVS38lViDqy vxtLX9JLU4YCCSkahAp/RyilUmvSHUjY2xf8oQDk8Yi4eZqkkNOxV6pvpxCp/aqzkHrLPA FFuXF2zBtIEX1m47yqaHHhD2oCYeGbc= 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 61C6F637D8; Thu, 27 Apr 2023 09:45:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 81EF4C433A0; Thu, 27 Apr 2023 09:44:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1682588702; bh=XbeJa7myqc5lpH0j9C1TtH8QoaXv86GZaE6uBALe91o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ky7i7k6QFq2LS1IXK40oMleXhg4LnfDdvsevFLmS7UpY27YL+99KpsBqi3jH1AuQw SdyZySmFtCCkARYqvAH5TtKyqh9ocd7BVhrKrfBcEBC+EF2GZczdoBmXhbQ4mJxOOY Gk4G1uyL3E2lHxht1iVcvlwOjvktkMe0bq5RPFB2BSMRpjPO6A86fCSWtBvOt2Eg/Q M7sw6lOP4cTZVlYIVeHal2TjI6E4L2tupt5Bxt++UEJUnYp0T3GV+g4eJiorOz6bxW no+xq90Xu8i81zvfS8Lwwp7ufo4ppDWR3SyhF3PRLCYImqxpjtPH+DeTpmpKSRU9Wc JW6K7RmLldkhg== Date: Thu, 27 Apr 2023 11:44:56 +0200 From: Christian Brauner To: Jeff Layton Cc: Alexander Viro , "Darrick J. Wong" , Hugh Dickins , Andrew Morton , Dave Chinner , Chuck Lever , Jan Kara , Amir Goldstein , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-nfs@vger.kernel.org Subject: Re: [PATCH v2 1/3] fs: add infrastructure for multigrain inode i_m/ctime Message-ID: <20230427-reinschauen-zwerge-a7f546c7d51a@brauner> References: <20230424151104.175456-1-jlayton@kernel.org> <20230424151104.175456-2-jlayton@kernel.org> <20230426-meerblick-tortur-c6606f6126fa@brauner> <07ce85763471a5964c9311792aa7e2f2d1696798.camel@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <07ce85763471a5964c9311792aa7e2f2d1696798.camel@kernel.org> X-Rspam-User: X-Rspamd-Queue-Id: 8561020020 X-Rspamd-Server: rspam01 X-Stat-Signature: yb8wq3bzgk6sa5eesrafkfgifinruh9j X-HE-Tag: 1682588704-75323 X-HE-Meta: U2FsdGVkX1+kaKpa6TH51UQC1RdjT6+tcn4pTnUE2DZ6c3JYQ5dWWbd0UU2Ejm8k/sP9pTtX7nkHL58fPIzuI+GOz9VAssL1P5X6GLKMkA3OgMiF6pPohUPYN+nTuKQT7Y4S1hivfzmagVZtxMeudAdSzzwhf4Ix2GAzu66ydw0h45DfjPqDF0z2SxTjBh5Vt+IsWRxTwo9GXe8NISxWjhTJloYDc3iW8Qo+1Ou2YWlsjamZEwStsvsPKcOz04rFOsOZ9lbqnaPJw04AOtd3cnspXI/fvyoX3zyH4U3NeLTebANv1ah9Shjbq38W+YRu6TOSwL5SPzyEemiLC23a5kffbetQxRs/1sFfKBZ0VTNiicdt6bV82/S3T932JLVxMddsJNhKnVzoSiq/8+LVdnj3tALgoor9G4SeL1VHFGxBKrJg/ULnttqNqDs5629i0PHNl+Td80BPl6p2Xmf6Say+r2WCndyEaVAoyufz0saftN4+jXv3zxQA4gfW5B5iDvsHKYontuh8U4KbYE1XwL8uETQF1HNbfXUKJkQvJ+Nke+ZC22ZqFdkTs6Vfvz7+8QeArZEeTVfxz6eSgSyKf5T6GSA9f2rD/MxNOUGcQeezIIgek6Z2+RGUpJmhkqv5CrNGWagJoWjgEIRmCFY/LWOR6iYgFWZU3UGirbPUwGLpmTGY0HnFI6gQbH+WqiKCBw6kZ+lOikXxspNq1WaUB2nWRKJMIbWzAO1RO6V4kWZQjH40Le/kbw3ianLMmNVUCEDy9fCo1fBmM4PmQXLI2fK6kRJnEqlBtGW2jq2EY8NJVGbyJawwQ6g8bL1D4OcC1NNjdXvX6sP8dKRf7xOt44v8JDDhcIcVg66vNqClPhlE0mCrb9T9LNWI26aynrQeqq/ot6DICnIPRmC5UeDuHNMLfvc9kdbJDmNlaZ12XbL4EIxcT8BD7QAdQsE61765rYp3/X7AUc9+QiFikrK 1wNmdM/E OL6WMZa3ul8LeYrh3bviWp25ZsEtRAmh4IA077gdr45wYOjVDsR1iYzMNpMgVvpaHJhVD+4qHt1kXVt8EaII7RGZ49GwsSxXczU0f0A8j0XEBfCX4EJ3G/XmuNIdM5td5TE/6V619kHx+UUzeqsXEA/nF0a/5BaqW3em+K3yeCrapiRRtllFTqmYXn6C2Eps9QAiG0BsAZQ1xZo/Uco1viYd6j6KF6bDIyYq8CFtxRg5ffU7p9bi2HcxkvH/ECsJ6ooNm3vvr74Tuxg+MPDHOgIB8H7fp1X3vQ5+73stAQr7vKmKJ0fJmX5sC2rR2LphhH1PG1aGxlBPKJds4X21O+aJUkDW9PqIcunJCAJcBGJJGco8QorQgoCcqzJj23OBFsz3xZDtSu/MltNQ= 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 Wed, Apr 26, 2023 at 05:46:25AM -0400, Jeff Layton wrote: > On Wed, 2023-04-26 at 08:53 +0200, Christian Brauner wrote: > > On Mon, Apr 24, 2023 at 11:11:02AM -0400, Jeff Layton wrote: > > > The VFS always uses coarse-grained timestamp updates for filling out the > > > ctime and mtime after a change. This has the benefit of allowing > > > filesystems to optimize away a lot metaupdates, to around once per > > > jiffy, even when a file is under heavy writes. > > > > > > 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 problems with timestamp granularity. Other > > > applications have similar issues (e.g backup applications). > > > > > > Switching to always using fine-grained timestamps would improve the > > > situation for NFS, but that becomes rather expensive, as the underlying > > > filesystem will have to log a lot more metadata updates. > > > > > > What we need is a way to only use fine-grained timestamps when they are > > > being actively queried: > > > > > > Whenever the mtime changes, the ctime must also change since we're > > > changing the metadata. When a superblock has a s_time_gran >1, we can > > > use the lowest-order bit of the inode->i_ctime as a flag to indicate > > > that the value has been queried. Then on the next write, we'll fetch a > > > fine-grained timestamp instead of the usual coarse-grained one. > > > > > > We could enable this for any filesystem that has a s_time_gran >1, but > > > for now, this patch adds a new SB_MULTIGRAIN_TS flag to allow filesystems > > > to opt-in to this behavior. > > > > > > It then adds a new current_ctime function that acts like the > > > current_time helper, but will conditionally grab fine-grained timestamps > > > when the flag is set in the current ctime. Also, there is a new > > > generic_fill_multigrain_cmtime for grabbing the c/mtime out of the inode > > > and atomically marking the ctime as queried. > > > > > > Later patches will convert filesystems over to this new scheme. > > > > > > Signed-off-by: Jeff Layton > > > --- > > > fs/inode.c | 57 +++++++++++++++++++++++++++++++++++++++--- > > > fs/stat.c | 24 ++++++++++++++++++ > > > include/linux/fs.h | 62 ++++++++++++++++++++++++++++++++-------------- > > > 3 files changed, 121 insertions(+), 22 deletions(-) > > > > > > diff --git a/fs/inode.c b/fs/inode.c > > > index 4558dc2f1355..4bd11bdb46d4 100644 > > > --- a/fs/inode.c > > > +++ b/fs/inode.c > > > @@ -2030,6 +2030,7 @@ EXPORT_SYMBOL(file_remove_privs); > > > static int inode_needs_update_time(struct inode *inode, struct timespec64 *now) > > > { > > > int sync_it = 0; > > > + struct timespec64 ctime = inode->i_ctime; > > > > > > /* First try to exhaust all avenues to not sync */ > > > if (IS_NOCMTIME(inode)) > > > @@ -2038,7 +2039,9 @@ static int inode_needs_update_time(struct inode *inode, struct timespec64 *now) > > > if (!timespec64_equal(&inode->i_mtime, now)) > > > sync_it = S_MTIME; > > > > > > - if (!timespec64_equal(&inode->i_ctime, now)) > > > + if (is_multigrain_ts(inode)) > > > + ctime.tv_nsec &= ~I_CTIME_QUERIED; > > > + if (!timespec64_equal(&ctime, now)) > > > sync_it |= S_CTIME; > > > > > > if (IS_I_VERSION(inode) && inode_iversion_need_inc(inode)) > > > @@ -2062,6 +2065,50 @@ static int __file_update_time(struct file *file, struct timespec64 *now, > > > return ret; > > > } > > > > > > +/** > > > + * current_ctime - 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/mtime change. > > > + * > > > + * For a multigrain timestamp, if the timestamp is flagged as having been > > > + * QUERIED, then get a fine-grained timestamp. > > > + */ > > > +struct timespec64 current_ctime(struct inode *inode) > > > +{ > > > + struct timespec64 now; > > > + long nsec = 0; > > > + bool multigrain = is_multigrain_ts(inode); > > > + > > > + if (multigrain) { > > > + atomic_long_t *pnsec = (atomic_long_t *)&inode->i_ctime.tv_nsec; > > > + > > > + nsec = atomic_long_fetch_and(~I_CTIME_QUERIED, pnsec); > > > + } > > > + > > > + if (nsec & I_CTIME_QUERIED) { > > > + ktime_get_real_ts64(&now); > > > + } else { > > > + ktime_get_coarse_real_ts64(&now); > > > + > > > + if (multigrain) { > > > + /* > > > + * If we've recently fetched a fine-grained timestamp > > > + * then the coarse-grained one may be earlier than the > > > + * existing one. Just keep the existing ctime if so. > > > + */ > > > + struct timespec64 ctime = inode->i_ctime; > > > + > > > + if (timespec64_compare(&ctime, &now) > 0) > > > + now = ctime; > > > + } > > > + } > > > + > > > + return timestamp_truncate(now, inode); > > > +} > > > +EXPORT_SYMBOL(current_ctime); > > > + > > > /** > > > * file_update_time - update mtime and ctime time > > > * @file: file accessed > > > @@ -2080,7 +2127,7 @@ int file_update_time(struct file *file) > > > { > > > int ret; > > > struct inode *inode = file_inode(file); > > > - struct timespec64 now = current_time(inode); > > > + struct timespec64 now = current_ctime(inode); > > > > > > ret = inode_needs_update_time(inode, &now); > > > if (ret <= 0) > > > @@ -2109,7 +2156,7 @@ static int file_modified_flags(struct file *file, int flags) > > > { > > > int ret; > > > struct inode *inode = file_inode(file); > > > - struct timespec64 now = current_time(inode); > > > + struct timespec64 now = current_ctime(inode); > > > > > > /* > > > * Clear the security bits if the process is not being run by root. > > > @@ -2419,9 +2466,11 @@ struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode) > > > if (unlikely(t.tv_sec == sb->s_time_max || t.tv_sec == sb->s_time_min)) > > > t.tv_nsec = 0; > > > > > > - /* Avoid division in the common cases 1 ns and 1 s. */ > > > + /* Avoid division in the common cases 1 ns, 2 ns and 1 s. */ > > > if (gran == 1) > > > ; /* nothing */ > > > + else if (gran == 2) > > > + t.tv_nsec &= ~1L; > > > > Is that trying to mask off I_CTIME_QUERIED? > > If so, can we please use that constant as raw constants tend to be > > confusing in the long run. > > Sort of. In principle you could set s_time_gran to 2 without setting > SB_MULTIGRAIN_TS. In that case, would it be correct to use the flag > there? Fair, then maybe just leave a comment in there. My main worry is going back to this later and then staring at this trying to remember what's happening...