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 B1941C3DA40 for ; Tue, 13 Jun 2023 19:47:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E2BB18E0003; Tue, 13 Jun 2023 15:47:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDB208E0002; Tue, 13 Jun 2023 15:47:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA34D8E0003; Tue, 13 Jun 2023 15:47:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id B6DDD8E0002 for ; Tue, 13 Jun 2023 15:47:57 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 79825405B3 for ; Tue, 13 Jun 2023 19:47:57 +0000 (UTC) X-FDA: 80898760194.15.1A0A692 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf25.hostedemail.com (Postfix) with ESMTP id AD65DA0005 for ; Tue, 13 Jun 2023 19:47:55 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Bzvqr3AB; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686685675; 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=sIv6TuYG7A66nvZvY3HxZmzTZ67+LlSq7cIGYg7ODXA=; b=G0hwBQ9m7FIpJpsblaGBgRKmvAzSWygIdp8whtcdYlbfsIp98mLtEvicOIT+OS9uJJPjkz VmpqrOMeHO4ciY5PEQ7VcLmn7kSU1MuiNVpKqteEHGbBrXUqrMs3fehIDHZ06UIOR7cPxl Ms1yHfSc40vJQMPeUX2TVN204n8zPao= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Bzvqr3AB; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686685675; a=rsa-sha256; cv=none; b=US3hBDIFc3Rt+ZeCPkAko0FK86Cbl7/pM/apvXGfdac2QYgoGPIn27pjZ+yyvEBV1O25CK lcAfra6vwa1eU+yfEFxdRCaWekOwGokeuPgw+sJ1Dyat9AJtn+btzP/uXwjxSDj/itTPPP TfrF3Qc7MyZsbGPoU3sgQgW7lKeJoro= 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 74849630A1; Tue, 13 Jun 2023 19:47:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 689F9C433F0; Tue, 13 Jun 2023 19:47:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1686685673; bh=Ih8TncHvHQnGjOpDZNZ3sjjTRueGP2vqBfz8GI8tuzM=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=Bzvqr3AB+cs7Dxe6cGNRzCtojYdBO9sWkNdcznkUfX9+ZDY25rOi+2u4X6VhgwrOx ohzqtKu5z6NzVdBIiq9UuiKh5r3YxLFfD13lPgSWk+X/9CT1KOQlStbaUZ+aMTqeQm Zv59qXw1fB68ELcdkT0vyiQlSiO88yVoQ6M8wAJ+VRka2OZ2ll9k/t/ptev5PqlA81 533DHLNEBBYgJL+ST2ZTHLORnjRBy6TL3jfL+P/Qpvg2wJc4/edGpmxXqJu805iPtw VImFMLd7MrTXUg8CeNHTp5sXPunoh5QsHtCVWD0aPCws9jG5dOyHohBjwDOMrI69JF 8sSWsZEKHd6CQ== Message-ID: <3b7a224853e2e0557d55e98f171f8b24999c040b.camel@kernel.org> Subject: Re: [PATCH v4 2/9] fs: add infrastructure for multigrain inode i_m/ctime From: Jeff Layton To: Jan Kara Cc: Alexander Viro , Christian Brauner , "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, 13 Jun 2023 15:47:50 -0400 In-Reply-To: <20230523101723.xmy7mylbczhki6aa@quack3> References: <20230518114742.128950-1-jlayton@kernel.org> <20230518114742.128950-3-jlayton@kernel.org> <20230523100240.mgeu4y46friv7hau@quack3> <20230523101723.xmy7mylbczhki6aa@quack3> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.3 (3.48.3-1.fc38) MIME-Version: 1.0 X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: AD65DA0005 X-Stat-Signature: rt95i8i6ouaa56p88jz7qxwa4o7xjbfa X-Rspam-User: X-HE-Tag: 1686685675-69752 X-HE-Meta: U2FsdGVkX1+XcfCfIrZZrC3Z2xyMtHE9DE4GwmISiwPqdo4IQ0xEKCGVIy2RvyjL6CjG37LPQevKjrYBqbdRs+b5t0MsZOXeWp4W2pOGppt7RCsb6PqH8KqZ0hnl4ZLQD/RAZkKzvdLlXAZPe//urA6TiA7Eoh0cdiZ6fHOUqd7GrbsYW6LUQ5HMDPoE7F4W114uml0IUdpCw0jFt0N+AwyOpCiJ6lQqDopdf8xeuxHnV5rODYB4lcrEBmUoLkEsvoZ7dpOzUagXzxwctAP9IaFHL8f6V5AHoPcyxodHo3PpgEZ9javI1YLM/jhqpeJwa3kBrkZU0eUZhx2RHPweR9RktZ9IVbt73TKlJ043I8oYRgWwWLcesSzXhYpWMNN10RlMtdOVnT5ZyI7xV8oUEUXP/29pxWnVJ+4LqWI2dAbrA2IBn3dv38cgZ99xBiC4DWAiW4Mo1BqO0mwVvCb6jwAvqmf09l/wFqJKRB3RtCvLEbblcxoZqI1PbBEJaK+xGVUS5X9aK3YaSz6WuzL1AhtxOREQ1z3CVAyzKunHCeFScFWv/TYwfBSmQBbGKMPX0Z9LEyiC+fbxumgTHgh4Bvr6iUYd+RGQnIoiJqAPkEpk3v55qJQKAMWJNS9TntaXUZArEz5RNMPdc3r/Zthgg8M+S67+Yb6DfqfztCOP1g2TUAHNokM8eOw+tGamxFq2OLaen3oOyyWqkwMdai8Mmv5W/kIMD4JUkQ0AOfWN+Hm9TJswz8H53rNAay7ieYpceHiNWF3tO1B5xEWFTKEgYxDxz97gRWT3+IbPYWEdmqqPVGCw192iPGWAGA/41g8xU8bSv0dGdOuhJIBHBHq12hLaGZ76gMBXvsp8JgVxPoxFhQI51aFlo7cC6Nu0a+edQmvsgMpE+DpkU/XRkKuNIqT5QwGSYqt3DQIEjDRee6KgM+4ETvF4vTz2gNPf9DO6ARVktW5jvbSBU33nKVL W7/1WqhG mHr9ZiIu/21+MI7Rjh/B3kxXUuP6jN2Cxk+csqREXEP5/XT9nMquYyGPlf8Fak5ghKtS/GLRLly5aaaGWpvJ05stUDsOAMAFZPkSEFD1MpJtACK9OvpVKTa5WjneK9Hnjq170ysYU1AboIiMKwKsZrU6p4ZeC+rXOO7SJ+7mveWhkxaxwsjPGOJkbJf88liNariocYqAlBDWmlxmJhNz92ZxjqbHjj56gx3UNhpRjO0Gs+c//4Ybi4SUwsHc5Fed+mKt17mChQMsyfKIeMU6dlZSE8OhYoAoAW6Pga9nAmrbhQ78ydLSSjA6EKx5DzXSlT83EITdhxubsyMcCEHUskcXykKKjfjHx2uH3AIcHIPtQOfZBLuCkKD4pts0EiB66OBcTWPCOG7sAnw8= 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 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 around 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 NFSv4= , a > > > lot of exported filesystems don't properly support a change attribute > > > and are subject to the same problems with timestamp granularity. Othe= r > > > applications have similar issues (e.g backup applications). > > >=20 > > > Switching to always using fine-grained timestamps would improve the > > > 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 they a= re > > > being actively queried. > > >=20 > > > The kernel always stores normalized ctime values, so only the first 3= 0 > > > bits of the tv_nsec field are ever used. Whenever the mtime changes, = the > > > ctime must also change. > > >=20 > > > Use the 31st bit of the ctime tv_nsec field to indicate that somethin= g > > > has queried the inode for the i_mtime or i_ctime. When this flag is s= et, > > > 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 opt > > > 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 the= y > > 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 bogu= s > > value (with QUERIED flag). This mistake is very easy to do. So I think = we > > should rename i_ctime to something like __i_ctime and always use access= or > > function for it. > >=20 > > 2) As I already commented in a previous version of the series, the sche= me > > with just one flag for both ctime and mtime and flag getting cleared in > > current_time() relies on the fact that filesystems always do an equival= ent > > 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 f= ine > > grained one. Filesystems often update timestamps like this but not > > universally. Grepping shows some instances where only inode->i_mtime is= set > > from current_time() e.g. in autofs or bfs. Again a mistake that is rath= er > > easy to make and results in subtle issues. I think this would be also > > 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_peek()= . > >=20 > > I understand this is quite some churn but a very mechanical one that co= uld > > be just done with Coccinelle and a few manual fixups. So IMHO it is wor= th > > the more robust result. >=20 > Also as I'm thinking about it your current scheme is slightly racy. Suppo= se > 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_nsec) > 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 assignment > =3D> we need not update ctime due to granularity although it was querie= d >=20 > One more reason to use explicit function to update inode->i_ctime ;) Thinking about this some more, I think we can fix the race you pointed out by just not clearing the queried flag when we fetch the i_ctime.tv_nsec field when we're updating. So, instead of atomic_long_fetch_andnot, we'd just want to use an atomic_long_fetch there, and just let the eventual assignment of inode->__i_ctime.tv_nsec be what clears the flag. Any task that goes to update the time during the interim window will fetch a fine-grained time, but that's what we want anyway. Since you bring up races though, there are a couple of other things we should be aware of. Note that both problems exist today too: 1) it's possible for two tasks to race in such a way that the ctime goes backward. There's no synchronization between tasks doing the updating, so an older time can overwrite a newer one. I think you'd need a pretty tight race to observe this though. 2) it's possible to fetch a "torn" timestamp out of the inode. timespec64 is two words, and we don't order its loads or stores. We could consider adding a seqcount_t and some barriers and fixing it that way. I'm not sure it's worth it though, given that we haven't worried about this in the past. For now, I propose that we ignore both problems, unless and until we can prove that they are more than theoretical. --=20 Jeff Layton