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 11FA9C7EE26 for ; Tue, 23 May 2023 11:15:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4721D900003; Tue, 23 May 2023 07:15:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 42375900002; Tue, 23 May 2023 07:15:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2EA3A900003; Tue, 23 May 2023 07:15:08 -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 1B8BC900002 for ; Tue, 23 May 2023 07:15:08 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 99ACD8076D for ; Tue, 23 May 2023 11:15:07 +0000 (UTC) X-FDA: 80821263054.01.3BBF4C7 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id 9277E14001C for ; Tue, 23 May 2023 11:15:05 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=kkOoNXps; spf=pass (imf26.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=1684840505; 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=J3dPaQzpggIsWVPiFD9LK6mpF/HsGn/OhR/I8ZTltFI=; b=e9oZy9RjfdFh3F+yZOcUzdPwDGLXhSrUz18u7zDi1iNtq/b3VajXILAkFJ5AjcW+xE42Uu dfUFpOgp8Y76/GGPSEaQDUj4I1C/MjVgjfDwHRHZq1ZKRy29yQbz++t7ea15SposWngcUm 5bZTLlW5hRZohnCSSN1KdzOp4pYcZC4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684840505; a=rsa-sha256; cv=none; b=t3Ylsh6ia/MBHLbF96Pqx1jKWH7weKS55oqhO9O9HTsbNOm7eHLqHCDLlkleUWyBuBm7jR rEsGvLQQdxBFh/qHjkaVlnIHL3kXmBJYTWIo7RDP76FK1hXMRgYPk+kUIK1JzmPor52zii LW80hZWPvBN+lyTWQoWN0+3aTS89Q38= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=kkOoNXps; spf=pass (imf26.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 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 86BDF60E33; Tue, 23 May 2023 11:15:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 86034C433EF; Tue, 23 May 2023 11:15:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684840503; bh=t7q3eN4BhkELHpTr8DtxIodckxoWL2ElEdGhZ8MHRXA=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=kkOoNXpsxbLfCboBMvGQ0TZ495A4EkCqbOAhscv082uZWwcELAtlQYSschubrdr7e s41Z0KfV1nRirJQ+AXcuUlwXITW7dG+QbnBqsROCo7fnGawbuOSiV1H/jac4LP62Px DgTNYFrlEMVh15PHU0jwip1npMemwKtmdPYAnnIQUUIw09adZ4/j4VXr9V1mp039FO 8zxW1TPfGEf3AnApU6fmuw7BVmsLam15SH3gzqm4qmy+oakOHWJck/MpVclOMnuvBR A+fjefks4366G9TneC0ktxH2ISCrf7eEdYApxB2Uw2EdkseBJW/MvR3OvEYUKuZ2ix 9aYZU2cyS5TdA== Message-ID: Subject: Re: [PATCH v4 2/9] fs: add infrastructure for multigrain inode i_m/ctime From: Jeff Layton To: Christian Brauner Cc: Jan Kara , Alexander Viro , "Darrick J. Wong" , Hugh Dickins , Andrew Morton , Dave Chinner , Chuck Lever , Amir Goldstein , David Howells , Neil Brown , Matthew Wilcox , Andreas Dilger , Theodore T'so , Chris Mason , Josef Bacik , David Sterba , Namjae Jeon , Steve French , Sergey Senozhatsky , Tom Talpey , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-mm@kvack.org, linux-nfs@vger.kernel.org, linux-cifs@vger.kernel.org Date: Tue, 23 May 2023 07:15:00 -0400 In-Reply-To: <20230523-undicht-antihelden-b1a98aa769be@brauner> References: <20230518114742.128950-1-jlayton@kernel.org> <20230518114742.128950-3-jlayton@kernel.org> <20230523100240.mgeu4y46friv7hau@quack3> <20230523101723.xmy7mylbczhki6aa@quack3> <20230523-undicht-antihelden-b1a98aa769be@brauner> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.1 (3.48.1-1.fc38) MIME-Version: 1.0 X-Rspamd-Queue-Id: 9277E14001C X-Rspam-User: X-Stat-Signature: 7q46wz1wgc57x9epc6u4k51qd3gdpwwa X-Rspamd-Server: rspam03 X-HE-Tag: 1684840505-111187 X-HE-Meta: U2FsdGVkX19N5ObRDbjYvkE01jgTsFPPZrgIfWjdrsGDXqFzqcHgtIWybx9LxC8tOkeNBqtWYXJA/ZT4sHCNW539ao9H8zESviW/qL9rv7+eNgII9rIAJzyl50DAzE5yxhoXp34W5RLC1TYf13VkqGKnhDjHBrtMHZQsIy6ke00q857zeQ5QSqGVfOheW7+BAI94uH5Lfn9PSS6zLByY9UAw2J4hqLviXk0IrrrzDpHUo5pdaPAQ2EpYEYuWtirPU9n2fyMPfOL/qVJc3IAjO3uITXOZHZ4uFx7rNnH6YfmF3wfMgc6goTLVSyl5g2SAFoaKV3I2FA0KOEZFDO1mpMtDNPTcMZpVc4DnqMk29cbmXWBorVqGho3aCWTXriaiZFhROl4og1AF2ggz0teelhxbQ1qBAYFsO3ZRXOWPpKaGc41mlg6hCFiiNX2VTt3GuwtSTpC/YbotB6kjFmRATXkzQEs4jIvCbLcqpnCZWwpALX6KV5lzZ1krPlAXRo6kczTseeFW+khY87YGC0WlPbk3Ju1BdoBuEQ5kHzNr1fMWcyRxP9aFwUkXwICgXGnXaj62fWLpWBlURlq2nlN9LoOIjhDDHR7R5IBSnv6Q0o+rcb+jUpCSUUZ+CGopf8cVybLnU5XTmThQdUp2DhK+XfPuk4Lo7DNqbtSq0COTDW2xxNw9BB9+cdKs5s/I6qaSk1zBnBycFqYL61yYwqQmEpX4yKzIH8qn5dEGOQGoAievNAG1y8ExD1BTf5dvg+9+2VP0dJWuhS5bpEvI3/PN9ZfmmZL4GqT1Hb0PYLga0hw9stmhJrpW96cRjsP1ihcraC5AEJziWQq8TW4zc+ujaSjZ6wQ4+KYzq54kpeVVHThD50sPFbHfZEyinq8WUZMRJzMv22FZkwhi2tmvAgjB++Tw7/KnA1q3UHVztIhAueGMIQsOIgziC4DLV5K54yerrcXKt9Ui0IN9HkkIc8d Dg47vQze EUzGTTDXts3RQhzdRv3Wm1t1rVwrb7hnX83t7h3tOxGc4qSO6cTFrToQY0o1zeySheryuFjYzS5s3PWmLOv4wgueM5KkNellC39RSPniuWLkaal5oGw+w5IpfRTfsfZOBO81mZd48O57uch+lZQIKrtJrWntMnoaHE8/TPK2hpdlr98xtXVSRbcsFsToD+tjMEbFPLD/zOWz/dnAVxOLe2u4yiI9QFKo0TZ6Nai/tWGRC105Su8gHYI5Yu6Z7N0YK1p9e9WbS8/T5b9Auz73XkeJ5HmCpLSzgku1ZNOZgM/fVsD0BpMZ+WhEqMPylClITh5mJitocK7pLCsqTTgaMEXw9zsE5d0VLkcl631NB+DtZH2O/dxzirFyWCS8n11jFmDElL8eY+5g4zY8= 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-05-23 at 13:01 +0200, Christian Brauner wrote: > On Tue, May 23, 2023 at 06:56:11AM -0400, Jeff Layton wrote: > > On Tue, 2023-05-23 at 12:17 +0200, Jan Kara wrote: > > > On Tue 23-05-23 12:02:40, Jan Kara wrote: > > > > On Thu 18-05-23 07:47:35, 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 metadata updates, down to arou= nd 1 > > > > > per jiffy, even when a file is under heavy writes. > > > > >=20 > > > > > Unfortunately, this has always been an issue when we're exporting= via > > > > > NFSv3, which relies on timestamps to validate caches. Even with N= FSv4, a > > > > > lot of exported filesystems don't properly support a change attri= bute > > > > > and are subject to the same problems with timestamp granularity. = Other > > > > > applications have similar issues (e.g backup applications). > > > > >=20 > > > > > Switching to always using fine-grained timestamps would improve t= he > > > > > situation, but that becomes rather expensive, as the underlying > > > > > filesystem will have to log a lot more metadata updates. > > > > >=20 > > > > > What we need is a way to only use fine-grained timestamps when th= ey are > > > > > being actively queried. > > > > >=20 > > > > > The kernel always stores normalized ctime values, so only the fir= st 30 > > > > > bits of the tv_nsec field are ever used. Whenever the mtime chang= es, the > > > > > ctime must also change. > > > > >=20 > > > > > Use the 31st bit of the ctime tv_nsec field to indicate that some= thing > > > > > has queried the inode for the i_mtime or i_ctime. When this flag = is set, > > > > > on the next timestamp update, the kernel can fetch a fine-grained > > > > > timestamp instead of the usual coarse-grained one. > > > > >=20 > > > > > This patch adds the infrastructure this scheme. Filesytems can op= t > > > > > into it by setting the FS_MULTIGRAIN_TS flag in the fstype. > > > > >=20 > > > > > Later patches will convert individual filesystems over to use it. > > > > >=20 > > > > > Signed-off-by: Jeff Layton > > > >=20 > > > > So there are two things I dislike about this series because I think= they > > > > are fragile: > > > >=20 > > > > 1) If we have a filesystem supporting multigrain ts and someone > > > > accidentally directly uses the value of inode->i_ctime, he can get = bogus > > > > value (with QUERIED flag). This mistake is very easy to do. So I th= ink we > > > > should rename i_ctime to something like __i_ctime and always use ac= cessor > > > > function for it. > > > >=20 > > > > 2) As I already commented in a previous version of the series, the = scheme > > > > with just one flag for both ctime and mtime and flag getting cleare= d in > > > > current_time() relies on the fact that filesystems always do an equ= ivalent > > > > of: > > > >=20 > > > > inode->i_mtime =3D inode->i_ctime =3D current_time(); > > > >=20 > > > > Otherwise we can do coarse grained update where we should have done= a fine > > > > grained one. Filesystems often update timestamps like this but not > > > > universally. Grepping shows some instances where only inode->i_mtim= e is set > > > > from current_time() e.g. in autofs or bfs. Again a mistake that is = rather > > > > easy to make and results in subtle issues. I think this would be al= so > > > > nicely solved by renaming i_ctime to __i_ctime and using a function= to set > > > > ctime. Mtime could then be updated with inode->i_mtime =3D ctime_pe= ek(). > > > >=20 > > > > I understand this is quite some churn but a very mechanical one tha= t could > > > > be just done with Coccinelle and a few manual fixups. So IMHO it is= worth > > > > the more robust result. > > >=20 > > > Also as I'm thinking about it your current scheme is slightly racy. S= uppose > > > the filesystem does: > > >=20 > > > CPU1 CPU2 > > >=20 > > > statx() > > > inode->i_ctime =3D current_time() > > > current_mg_time() > > > nsec =3D atomic_long_fetch_andnot(QUERIED, &inode->i_ctime.tv_nse= c) > > > nsec =3D atomic_long_fetch_or(QUERIED, &inode->i_ctime.tv_nsec= ) > > > if (nsec & QUERIED) - not set > > > ktime_get_coarse_real_ts64(&now) > > > return timestamp_truncate(now, inode); > > > - QUERIED flag in the inode->i_ctime gets overwritten by the assignme= nt > > > =3D> we need not update ctime due to granularity although it was qu= eried > > >=20 > > > One more reason to use explicit function to update inode->i_ctime ;) > >=20 > > When we store the new time in the i_ctime field, the flag gets cleared > > because at that point we're storing a new (unseen) time. > >=20 > > However, you're correct: if the i_ctime in your above example starts at > > the same value that is currently being returned by > > ktime_get_coarse_real_ts64, then we'll lose the flag set in statx. > >=20 > > I think the right fix there would be to not update the ctime at all if > > it's a coarse grained time, and the value wouldn't have an apparent > > change to an observer. That would leave the flag intact. > >=20 > > That does mean we'd need to move to a function that does clock fetch an= d > > assigns it to i_ctime in one go (like you suggest). Something like: > >=20 > > inode_update_ctime(inode); > >=20 > > How we do that with atomic operations over two values (the tv_sec and > > tv_nsec) is a bit tricky. I'll have to think about it. > >=20 > > Christian, given Jan's concerns do you want to drop this series for now > > and let me respin it? >=20 > I deliberately put it into a vfs.unstable.* branch. I would leave it > there until you send a new one then drop it. If we get lucky the bots > that run on -next will have time to report potential perf issues while > it's not currently causing conflicts. Sounds good to me. Thanks! --=20 Jeff Layton