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 84304C76196 for ; Tue, 11 Apr 2023 15:07:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F2D9D6B0072; Tue, 11 Apr 2023 11:07:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EDDDA6B0075; Tue, 11 Apr 2023 11:07:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DA650900002; Tue, 11 Apr 2023 11:07:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id CDBDE6B0072 for ; Tue, 11 Apr 2023 11:07:24 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 09FFA140E0D for ; Tue, 11 Apr 2023 15:07:23 +0000 (UTC) X-FDA: 80669438808.23.0B05199 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf12.hostedemail.com (Postfix) with ESMTP id 774DA40004 for ; Tue, 11 Apr 2023 15:07:20 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=faSha008; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681225640; 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=stWLwJ5KYk0thKxYlpyk95j48I7/69KMWR4AexetXm0=; b=E6QOCHfnEk/CkV+aDeomkvdFRqwYqEArk+rw+qcbN4lTu929hpJ7QySMHxbRcvwOMBMTae ZWTEN/rqF1FGK0z9Ri1QFXX+PIOWKVZ4OQl6w1rWyVjID0jtX+IOH/mDCmyIw8sIETmMkF tfRmN8WRv+4/XqYFatss/E3fCNJmrfE= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=faSha008; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf12.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681225640; a=rsa-sha256; cv=none; b=TX1qNq9dyCfNoXCLZjBsBp4EBHYm0DG8eA9NXbuSs23zooGhvArHLgNqCWJykWo8GLb456 eX6PpBDe3iK1Zdc6J2Z5KiADqn8rhhvDOFO4reBjalJYD06Y0GdPa11EW0MJw25G04c4+3 knjABseecIBM3x0ZuZcx7G1NwwIhbuo= 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 7BFB461F86; Tue, 11 Apr 2023 15:07:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 04C16C433EF; Tue, 11 Apr 2023 15:07:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681225638; bh=sZyOp9FyHjFY8Ijg56XZndRZkaDLI3AILJspX6fqzyE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=faSha008mhaP7F6Mb6PjpHGM8UWH6bJuVHhtAaUybY1IhSeqSDKwk3Xkl01Hf6wgq vtHVzTLvioS4dReDgA0w8F21rX3+s+KfJiPan+Y1ABiR/KYdfaX60GwQyd3DZ0RrfN nH45POKQrFDpdS7deCs4xmnqrYnS9fHzvJ3jpJYcbNbI6OseG4pUlgskvhFBzCldTv UwgyDWlC8onNuaXfWQ2Nv3MPknICoLOtVUM1iDWWzX7oy/Zp3zm6C3CyyLF5gsOVAE nocFI4Ul9FXxGMGoh/4r4BOHNnGMzGIIDqYgPmzigBwpxoiOjCBVb8eDfe6dJ7wlTN wbMzsf3CBFGZA== Date: Tue, 11 Apr 2023 17:07:12 +0200 From: Christian Brauner To: Jeff Layton Cc: Alexander Viro , "Darrick J. Wong" , 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 Subject: Re: [RFC PATCH 1/3][RESEND] fs: add infrastructure for opportunistic high-res ctime/mtime updates Message-ID: <20230411-unwesen-prunk-cb7de3cc6cc8@brauner> References: <20230411143702.64495-1-jlayton@kernel.org> <20230411143702.64495-2-jlayton@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20230411143702.64495-2-jlayton@kernel.org> X-Rspamd-Queue-Id: 774DA40004 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: eu9x7jr4sqcrkai3wesxsah68dne193n X-HE-Tag: 1681225640-547644 X-HE-Meta: U2FsdGVkX1/UngDveXQ7dvSYtZhcOyQlk3tC2xdQoeAcR8jinu/rw4dDMz1EolLlaIXJmiMOGmMVyor7HsAASKAiX906ZjimhvXzhQktGXrJs6GsiAQx5CNCIelcCGv1P5098QswxBqYUIIlt0EnU7wKt99V6CqlkA9Rfy/HWSh7evwaWiQhSV/zAAkQiRaWC+s3bxsAePxFdDFhVNkYuC64CvWaFYBOcAivNfdwIw4HpeZJAak+g68hpKGj3cikOcv1GVqfa3FX9V4YUCMEb8T4bUkbx1XmParFMoBXK75xJKdWVew1LXIvTgNm4sKwR8RbMuKOTMcbaBgkDXijkB0tfQGJJT6e63zuhp+xpn/UDwESNeOg0p5Wgwf0ewtUl/fJBV44plTx6diR83OzKdwPRMQ7xm7VQ/NaIN1CGEQB1SU9APfUEw5soVCRq4lphz6R0cD0la4QymDT8PzsJsaOyyGjf4hBHD4bKOpzC3LojndQuL2V72AVtAMkWKZj/4Ywk+iAMcEEiQ67JzY6305mnytM8ieB8pXUNc9hzPOs9sTACwAPDYYXm5Z8gLdKpwNQTgJetjHuCRkoTOaxy37uFUDLTU4/7x7l2Fyq8the5ITN8PvrGPV/ST3cVPH04hV/czwRlq2sYONtCj2IyhCB0UuB5M0KLoNCQAPnnXXrQY4ydddlPbHQL7oDkttNwoPRDNgM+yBs1NrhdiubFS/0GvmTm2ykjK2cFhirTGE0CzRTh2W4E40tY3YjxNoFZD7B5RBJJvUXpo0fGn3tT+2/SWpb5zc/pjykRaJiL50do5UlSvam+gCb0Kg+8lbU3kvFEqAfH4iKV6bODj2pOcAmuHwceKNNijI9txmoSrid+wmX8ptAmeue2QO8Ivvqb+Ndt7qTOUzSRFRs2RZlJfFP5ivjdSVYvww/8Xrnnu75J7FsnamZoSHK14Kv5gZ3w2DRypoLbuYRRr2He73 Sn3OebhF kzgJ9pAju9152wB6045M6dS6+44lgVfAAZIqJOIdEqqUpYzYBxEC5D+2UROqaDPAqg/AbaHZoUXdegkD/a/0DDFky+fRXXLeecRiYHA9X/6cvBGRf6x5BgQssgEGBmSUjiXPdn2kDRMPOunb0kxIRk1SoQHTpxoKrzJaXgi/zJtwVBWAR4NbdGuXJAwkmABSzxzAsSQ3+x8lov6q0D77mbPMasJ4VDHhvNETbb/Zk21+7QtDPorrYRRuHIAsxRaAk1PAiC6XS0h6YRpQoeSvlxFCpK7dNaHDIfCNETVJUAQ2bvo7dNX1s201lyRbHkEuWgh8sFu4y5auNlDSL8+My32hg+DlFExhXBVEP8h21dAfiDaU/GL6CGNJkCtEDYzBDM3iV3OpG4Cm3zG/EShyG++xr9QTvlUIlldTHkT3a6tpUk7o= 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, Apr 11, 2023 at 10:37:00AM -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 metadata updates. > > 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). > > 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. > > This patch grabs a new i_state bit to use as a flag that filesystems can > set in their getattr routine to indicate that the mtime or ctime was > queried since it was last updated. > > 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. > > 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. > > 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(-) > > 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; > } > > +/** > + * 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 recently > + * 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 truncation. > + */ > +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__); How would this happen? Seems weird to even bother checking this. > + 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 &= ~I_CMTIME_QUERIED; > + spin_unlock(&inode->i_lock); > + } else { > + ktime_get_coarse_real_ts64(&now); > + } > + > + return timestamp_truncate(now, inode); > +} > +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 = file_inode(file); > - struct timespec64 now = current_time(inode); > + struct timespec64 now = current_cmtime(inode); > > ret = inode_needs_update_time(inode, &now); > if (ret <= 0) > @@ -2109,7 +2145,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_cmtime(inode); > > /* > * 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, struct inode *inode, > } > EXPORT_SYMBOL(generic_fillattr); > > +void fill_cmtime_and_mark(struct inode *inode, struct kstat *stat) > +{ > + spin_lock(&inode->i_lock); > + inode->i_state |= I_CMTIME_QUERIED; > + stat->ctime = inode->i_ctime; > + stat->mtime = inode->i_mtime; > + spin_unlock(&inode->i_lock); > +} > +EXPORT_SYMBOL(fill_cmtime_and_mark); So that means that each stat call would mark an inode for a high-resolution update. There's some performance concerns here. Calling stat() is super common and it would potentially make the next iop more expensive. Recursively changing ownership in the container use-case come to mind which are already expensive.