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 D2FDFC3DA49 for ; Thu, 11 Jul 2024 15:09:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B2F76B009D; Thu, 11 Jul 2024 11:09:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 062946B009E; Thu, 11 Jul 2024 11:09:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E450F6B009F; Thu, 11 Jul 2024 11:09:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C33766B009D for ; Thu, 11 Jul 2024 11:09:24 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 69D4616060E for ; Thu, 11 Jul 2024 15:09:24 +0000 (UTC) X-FDA: 82327805448.23.F82D01F Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id B2C89140025 for ; Thu, 11 Jul 2024 15:09:22 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="Ev44/ZTk"; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf26.hostedemail.com: domain of djwong@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720710538; a=rsa-sha256; cv=none; b=HK5Bgj4TyBkw1bKPgxaUDT6uvquW1l5h7jI4imWMW4DJgQLrAeA4AFVciw3EJu+AYkl+/3 BUCNjY39bLctKdw+Rb6KV7m1N7DVMBxN7skbeSPhGApW3PsQ0Ro4E5Om6Ag9sMSWeqRLl1 QKWp6QqlwmBJLYoZLvLX0NR0mWrua3w= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="Ev44/ZTk"; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf26.hostedemail.com: domain of djwong@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720710538; 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=nh7vxCqeubn0hCXrYg/JTXePsyKRWUsaBnt2VxkGP1g=; b=6mbtwCzWcrIThXLBsxTjWHvs5zRoZx+DWFu75p24VmdMavwdHJjf+ZCU6Z3haJbehvlIRg +ibb8h2kJ7XPL3KxKaR2TVhZT4++w5JZlfMtjVK3Yghzv09k8zbZMVIufLcjYGdxk2lyr3 A8WMQHSOiJXfKwoyJUjRXGm+G/oG6Ck= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B60C961D2E; Thu, 11 Jul 2024 15:09:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5D221C116B1; Thu, 11 Jul 2024 15:09:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720710561; bh=eATJeGRQX6K8fg/DNPVE9DpTSEVJo8tgmyL1kR7ePkA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ev44/ZTkKHFe7RxCRO1rosrnR5PWIL44jnysmBLNGBFSgFn8QkBx19pwS2RPzDpdt 1w9hnlQBnaS9k4h0lEuTTlHKNskAQPdJkOAn4TCYqhd4NAfbUlWj2SF7SNWoT5m3cO RbXezDly7f61sLovkgZojrKnRIH2AUTdT/fqs3WNkWDLZbyJgV/JxY1sRyVbZaoKBu ErCxjcQp++jmQDFfNoVXzcmNIF5A+F59cIdmuEO8iUNm/Ci6RBBTyscf7XiS+/gGRz iIl9s/npTiRkZQKUijmvbqHinlzdFe9TYTkON21xPWJR9eVmXwyByTolueIda3Nbin /ffOm2lA+0xgQ== Date: Thu, 11 Jul 2024 08:09:20 -0700 From: "Darrick J. Wong" To: Jeff Layton Cc: Alexander Viro , Christian Brauner , Jan Kara , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Chandan Babu R , Theodore Ts'o , Andreas Dilger , Chris Mason , Josef Bacik , David Sterba , Hugh Dickins , Andrew Morton , Jonathan Corbet , Dave Chinner , Andi Kleen , Christoph Hellwig , Uros Bizjak , Kent Overstreet , Arnd Bergmann , Randy Dunlap , kernel-team@fb.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-mm@kvack.org, linux-nfs@vger.kernel.org, linux-doc@vger.kernel.org Subject: Re: [PATCH v5 6/9] xfs: switch to multigrain timestamps Message-ID: <20240711150920.GU1998502@frogsfrogsfrogs> References: <20240711-mgtime-v5-0-37bb5b465feb@kernel.org> <20240711-mgtime-v5-6-37bb5b465feb@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240711-mgtime-v5-6-37bb5b465feb@kernel.org> X-Rspamd-Queue-Id: B2C89140025 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 3j8zeychrxba6remdwp6z696b6sdkdy7 X-HE-Tag: 1720710562-192525 X-HE-Meta: U2FsdGVkX1/EnHP+QbaTsuVJ4SNuHLbq6wNYOjL+3D8Sbprg0j1VKU34zKBkf1Gao90NrWclL0GFEpAorHy7m8KZuSqeH5Tje/blaMu3EyW39UKNE+LENPS8239P4SUl2Pic4Q33NbHNKDQ34coYUjR8KDuBDGzTXcrkqT22VmMU4AlUVqDBI2L8kh7R1lyPR/B91J5b5RsO3ky/cb07Lqw5kFmvcj4EIw1s3VHemQzEdooGmJeO894rISfpB55SpGuqy1oAyp4ezXVQ+qdI6ZhrbqXMsEqlkUgJaOet+1SjlNwM14qAKsHquOE8bdyG2BIao/Z6S6Wp3lJumf4coqOXe9PlwiG7UZfgaaDNaX+efKOSCBlrTHiPMcQyBih5vD8P9Evhe/jWt9j3fthexJbHWQJHJ4rmBiBxFjRCcPHhxz09bFUbVZdfEH5RFQ4UKT5p7fvQXtT751SvKOrvgDLVIU+tWQmc9lmWOkADPvDLAnow2BIYg9RXL9FW7Znk+HkEsJRpAHMemAyiBJhMSJ52TZ0m8RiP8+h3lTkRCuATQHkJWgRFHuWmBpxO6VaVVUfiq2KtIzz1pkfCxYqirg326a03rtBwME9+QMGIu7DwOA3dDdpxpOuupuLwJ7t2/8Ge9oh+8IWneq9IYOAAujdQgz9ddTOVk7kHXL/CRudMK+8BAqZUiTIVv6MexFEjyoTBTPL9C7CI2wqwdeHpBjHjbSBHc7dtPVdELOfzo4ua+Cruywvq8StzLknKvHgSebXT3BAs1V7GRAXBlZCBmpHpLq/VSZ9G8ZTkEQxM473MtERyCMkR5z30hbFPp/vQjHFAnFywDefl1IJDtQDA+WePvZV4TqXS3T0b1Fgo/puYiLQ+dthDeOLOdxV8kc2wp5lXpvyd+6mFiVwPRmeY/RPICEEmmh3qdD0Slv8cvWMzpG8SjHQ5T7OcFPtGEXPxwXapdorK4WsIlLNg0s8 jA4WHiFI bjOXDAwJUamQE76Fz9eYSmBcPopJY52VcTGnyBqPQ1L5HLsgd4VSha5MNZpjps3OBXS/0TKive8n+C3VKYzuOHtWET/JMlttv0ipujK4fJtdGfDuEOw4FIyAU3kRZvI7sxI6naUOIbAwAyQrbObAAGKurouem32FT5nQ/6XdzQ4IQPsUMRnAt/8czxUA7tCN6AlIGvZab48ek3vf584xBEzb1CLiGKb9xCH/cTETWV3WPIgwtDpZmdBXHIYH+r5c4tSLZD017Ybysp4nelx/phpAAEw== 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: List-Subscribe: List-Unsubscribe: On Thu, Jul 11, 2024 at 07:08:10AM -0400, Jeff Layton wrote: > Enable multigrain timestamps, which should ensure that there is an > apparent change to the timestamp whenever it has been written after > being actively observed via getattr. > > Also, anytime the mtime changes, the ctime must also change, and those > are now the only two options for xfs_trans_ichgtime. Have that function > unconditionally bump the ctime, and ASSERT that XFS_ICHGTIME_CHG is > always set. > > Finally, stop setting STATX_CHANGE_COOKIE in getattr, since the ctime > should give us better semantics now. Following up on "As long as the fs isn't touching i_ctime_nsec directly, you shouldn't need to worry about this" from: https://lore.kernel.org/linux-xfs/cae5c28f172ac57b7eaaa98a00b23f342f01ba64.camel@kernel.org/ xfs /does/ touch i_ctime_nsec directly when it's writing inodes to disk. >From xfs_inode_to_disk, see: to->di_ctime = xfs_inode_to_disk_ts(ip, inode_get_ctime(inode)); AFAICT, inode_get_ctime itself remains unchanged, and still returns inode->__i_ctime, right? In which case it's returning a raw timespec64, which can include the QUERIED flag in tv_nsec, right? Now let's look at the consumer: static inline xfs_timestamp_t xfs_inode_to_disk_ts( struct xfs_inode *ip, const struct timespec64 tv) { struct xfs_legacy_timestamp *lts; xfs_timestamp_t ts; if (xfs_inode_has_bigtime(ip)) return cpu_to_be64(xfs_inode_encode_bigtime(tv)); lts = (struct xfs_legacy_timestamp *)&ts; lts->t_sec = cpu_to_be32(tv.tv_sec); lts->t_nsec = cpu_to_be32(tv.tv_nsec); return ts; } For the !bigtime case (aka before we added y2038 support) the queried flag gets encoded into the tv_nsec field since xfs doesn't filter the queried flag. For the bigtime case, the timespec is turned into an absolute nsec count since the xfs epoch (which is the minimum timestamp possible under the old encoding scheme): static inline uint64_t xfs_inode_encode_bigtime(struct timespec64 tv) { return xfs_unix_to_bigtime(tv.tv_sec) * NSEC_PER_SEC + tv.tv_nsec; } Here we'd also be mixing in the QUERIED flag, only now we've encoded a time that's a second in the future. I think the solution is to add a: static inline struct timespec64 inode_peek_ctime(const struct inode *inode) { return (struct timespec64){ .tv_sec = inode->__i_ctime.tv_sec, .tv_nsec = inode->__i_ctime.tv_nsec & ~I_CTIME_QUERIED, }; } similar to what inode_peek_iversion does for iversion; and then xfs_inode_to_disk can do: to->di_ctime = xfs_inode_to_disk_ts(ip, inode_peek_ctime(inode)); which would prevent I_CTIME_QUERIED from going out to disk. At load time, xfs_inode_from_disk uses inode_set_ctime_to_ts so I think xfs won't accidentally introduce QUERIED when it's loading an inode from disk. --D > Signed-off-by: Jeff Layton > --- > fs/xfs/libxfs/xfs_trans_inode.c | 6 +++--- > fs/xfs/xfs_iops.c | 10 +++------- > fs/xfs/xfs_super.c | 2 +- > 3 files changed, 7 insertions(+), 11 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > index 69fc5b981352..1f3639bbf5f0 100644 > --- a/fs/xfs/libxfs/xfs_trans_inode.c > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > @@ -62,12 +62,12 @@ xfs_trans_ichgtime( > ASSERT(tp); > xfs_assert_ilocked(ip, XFS_ILOCK_EXCL); > > - tv = current_time(inode); > + /* If the mtime changes, then ctime must also change */ > + ASSERT(flags & XFS_ICHGTIME_CHG); > > + tv = inode_set_ctime_current(inode); > if (flags & XFS_ICHGTIME_MOD) > inode_set_mtime_to_ts(inode, tv); > - if (flags & XFS_ICHGTIME_CHG) > - inode_set_ctime_to_ts(inode, tv); > if (flags & XFS_ICHGTIME_CREATE) > ip->i_crtime = tv; > } > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index a00dcbc77e12..d25872f818fa 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -592,8 +592,9 @@ xfs_vn_getattr( > stat->gid = vfsgid_into_kgid(vfsgid); > stat->ino = ip->i_ino; > stat->atime = inode_get_atime(inode); > - stat->mtime = inode_get_mtime(inode); > - stat->ctime = inode_get_ctime(inode); > + > + fill_mg_cmtime(stat, request_mask, inode); > + > stat->blocks = XFS_FSB_TO_BB(mp, ip->i_nblocks + ip->i_delayed_blks); > > if (xfs_has_v3inodes(mp)) { > @@ -603,11 +604,6 @@ xfs_vn_getattr( > } > } > > - if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) { > - stat->change_cookie = inode_query_iversion(inode); > - stat->result_mask |= STATX_CHANGE_COOKIE; > - } > - > /* > * Note: If you add another clause to set an attribute flag, please > * update attributes_mask below. > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 27e9f749c4c7..210481b03fdb 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -2052,7 +2052,7 @@ static struct file_system_type xfs_fs_type = { > .init_fs_context = xfs_init_fs_context, > .parameters = xfs_fs_parameters, > .kill_sb = xfs_kill_sb, > - .fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP, > + .fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP | FS_MGTIME, > }; > MODULE_ALIAS_FS("xfs"); > > > -- > 2.45.2 >