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 3E496CF64BE for ; Tue, 1 Oct 2024 13:32:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ACA5D280086; Tue, 1 Oct 2024 09:32:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A79C7280068; Tue, 1 Oct 2024 09:32:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 91AA0280086; Tue, 1 Oct 2024 09:32:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 73BE3280068 for ; Tue, 1 Oct 2024 09:32:29 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D15EAAC966 for ; Tue, 1 Oct 2024 13:32:28 +0000 (UTC) X-FDA: 82625122776.05.3F871F3 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf24.hostedemail.com (Postfix) with ESMTP id 8D003180025 for ; Tue, 1 Oct 2024 13:32:25 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=rDj2Daeg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="B1A5Czx/"; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=rDj2Daeg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="B1A5Czx/"; spf=pass (imf24.hostedemail.com: domain of jack@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727789418; 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=EJyVh7xpKFUerqgpvOUGQtwMuhiG+vMZUA+eKVSBOeQ=; b=2TiSl+GlpLTmMwZaPkyTklCYoEx5D4mBEP9nP+MnCZglUa2FND8YWlH8t+TcTMO5QL5wfE 6TS861jaJ0HMIaXGDcIK05plV609B4uRndCOWvHX7frFlaKxp63r30PiIyE8PzJRV5suC7 DXhboOu0knr/SrxtUxH2X98U06ATXOc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727789418; a=rsa-sha256; cv=none; b=IHD+I8jkK9x4Sh1RbGniS24AUGxqNV9HCgo2ipBofaLS6GKxRuiITPD8TEtFRrH+/pyAo5 EylkePPd8dfF49UsvYzAmNPkm9jIfMZysvIzJx/WaJM7Fk7DdI4tW+UsOaJP0ChPNXh6Sv 17wvR/jnoOn8mL2updSnWMBjO+pDg1U= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=rDj2Daeg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="B1A5Czx/"; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=rDj2Daeg; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="B1A5Czx/"; spf=pass (imf24.hostedemail.com: domain of jack@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id BC9501F813; Tue, 1 Oct 2024 13:32:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1727789543; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EJyVh7xpKFUerqgpvOUGQtwMuhiG+vMZUA+eKVSBOeQ=; b=rDj2Daegw8acNetwuj+igR8urk35s9IZ5Ei4I5Xxg1Hf1yqHsGh07lzo6akylF2rlreVuA RBAeL6aZkGoF0vKutQUZhkX3qKm/b1/jlPlXDc2TU6/f+CeVzyg5lpcXpqHqMArMLoBpqB v9XWD2kMJCRuN9Moac3CaUXQnIsYDQE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1727789543; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EJyVh7xpKFUerqgpvOUGQtwMuhiG+vMZUA+eKVSBOeQ=; b=B1A5Czx/JU851NEzyiwPym53Dc/Uk7kviKL8OOcS1KkhL/cvrtNrUAxf40xEk6CgQKiBQn jRfTdeTqntkjpyCA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1727789543; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EJyVh7xpKFUerqgpvOUGQtwMuhiG+vMZUA+eKVSBOeQ=; b=rDj2Daegw8acNetwuj+igR8urk35s9IZ5Ei4I5Xxg1Hf1yqHsGh07lzo6akylF2rlreVuA RBAeL6aZkGoF0vKutQUZhkX3qKm/b1/jlPlXDc2TU6/f+CeVzyg5lpcXpqHqMArMLoBpqB v9XWD2kMJCRuN9Moac3CaUXQnIsYDQE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1727789543; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EJyVh7xpKFUerqgpvOUGQtwMuhiG+vMZUA+eKVSBOeQ=; b=B1A5Czx/JU851NEzyiwPym53Dc/Uk7kviKL8OOcS1KkhL/cvrtNrUAxf40xEk6CgQKiBQn jRfTdeTqntkjpyCA== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id AEA3713A73; Tue, 1 Oct 2024 13:32:23 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id vHWbKuf5+2ZwDwAAD6G6ig (envelope-from ); Tue, 01 Oct 2024 13:32:23 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 6A189A0881; Tue, 1 Oct 2024 15:32:19 +0200 (CEST) Date: Tue, 1 Oct 2024 15:32:19 +0200 From: Jan Kara To: Jeff Layton Cc: John Stultz , Thomas Gleixner , Stephen Boyd , Alexander Viro , Christian Brauner , Jan Kara , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Jonathan Corbet , Randy Dunlap , Chandan Babu R , "Darrick J. Wong" , Theodore Ts'o , Andreas Dilger , Chris Mason , Josef Bacik , David Sterba , Hugh Dickins , Andrew Morton , Chuck Lever , Vadim Fedorenko , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v8 04/12] fs: handle delegated timestamps in setattr_copy_mgtime Message-ID: <20241001133219.aenkpa5ydeefqqyd@quack3> References: <20241001-mgtime-v8-0-903343d91bc3@kernel.org> <20241001-mgtime-v8-4-903343d91bc3@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241001-mgtime-v8-4-903343d91bc3@kernel.org> X-Stat-Signature: gbpa69azrtyeya8rhdgmy9ezee9tjy4y X-Rspamd-Queue-Id: 8D003180025 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1727789545-555670 X-HE-Meta: U2FsdGVkX193uAa0vVZMirdDGuufuuY/wC3TxAcM1uqI1w25e/tkZlWpQRm/pFxp58thF8resjHdqQby/sX+U805zDmikDoM3tJCE2ZHKwRZWclThMViHwgrh4PSPua7ToNEGOCcBj28hLRfeGjGMyTV4nzAtLcFkRAZMMx91SupWdH8pWLdstPd8OlWMRmWPPLs1JQD8TCd6aDqNGm/D1rjEjECxjfoNsGtjGd8l/xzOhuWuDjAS+yiaUZd5Ea0h09DaArBz1q9pwjNG3jpI9HS28BXAJYSMohLVXS/O2/0by6z1V2imuqVeOinyD1tJdZvx7izFsLlx7CHpTHWda+qtQdFVGJmDJAlyXj+Rb8lAD75EwbXBaejEGeadWgUEi8/wexMfTdRWE9JoWCmZgrZJj7pdUW0AWJ6ptEZuES/wZAagn9p+XUryq0tOapQycqzcCzy2HrDbM66mKmbMvwS5m5JTXw0GSNEudUCTGbyuVMfkTGiWQs3LbNGHWT3AiwIgeJ9d+CO/F86NKNQEp2vgBzjvyyUF8eGIotUPIofZ9NFHGw0l1X88DPzy3BkQvUs97NOSXhTT1qXoiv6yibAw4xRbQ7zlPRm7BpO44gce+UDhQaHeO5LAPNn//W367E5EYSh2UyNr1dfijuMRbPpxg7wM3rzKq1t6M13OcI1ybrBmUGEYMtl+V0Hr4KJj3qeytUpdRpMAaNhTMkBK1/1yDcuswzrDWkLk81uLaLffcpd8R96WwXGAOXDaPQBzOI9RE+hHllIx7aEFnqlolCSYv/1gWf8vNb2zADk6ViqR8ZhWmspBh/yYLCcfaus/Tq3r7yU5kFgO6NcZqKRLh3qC/oWS9DJhXJM42SvWxYyqrxDk57KY+YyR5BqgvUic/5XrKBto7XlycKBRBD+bYZ5vofj8RyXf1tpRz4POJFdlaaabbuxwFJQ6/XWQ42xrWwSzGG+t1kJa+oH+0U q44zao0e dwXu2E8wvg9L9lkeOx4nhQKq7V0R5DCN+qutMkVeI9LgzVVjwTyTijjSE0KCw8c4WrecPjwlr0lNsV6oBAL1m8xR1cC9ZLm2vnplBt/JpxxoOjAbR6LIp2JXeH8+r9OJu0R1Cq1nhZIpmL4qc5wSwBBEWOYXWJAknAdHNwX/RWRtSvbFSSTOUNFu3QKrdLkUUDoyeTJaiVepF8r4xFVvTqgFSPD5A9pFZ6m7IHgA+KLFr8S69rvGLH6k0gOyj85GWeT3qS4JZS0yiAcBYZpGCrInP7H2rmn6JHYn7Iz52sQnCkt+cdVRZMXU8KILwApDLTybsXErX2zcarRhjciX3zwfZWCbhWu33FLAbcC4Jy46A1K+qTFNbGGvt3j0x2HXS/MQSnoe83rh7HfeTdxlo7d9dG71LpDHWl3+7qlLm8UraFzzq0KY5r41JcV8VKa6XuC7H4Ro5jvomLFK+jgOqIOnxdQ== 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 Tue 01-10-24 06:58:58, Jeff Layton wrote: > When updating the ctime on an inode for a SETATTR with a multigrain > filesystem, we usually want to take the latest time we can get for the > ctime. The exception to this rule is when there is a nfsd write > delegation and the server is proxying timestamps from the client. > > When nfsd gets a CB_GETATTR response, we want to update the timestamp > value in the inode to the values that the client is tracking. The client > doesn't send a ctime value (since that's always determined by the > exported filesystem), but it can send a mtime value. In the case where > it does, then we may need to update the ctime to a value commensurate > with that instead of the current time. > > If ATTR_DELEG is set, then use ia_ctime value instead of setting the > timestamp to the current time. > > With the addition of delegated timestamps we can also receive a request > to update only the atime, but we may not need to set the ctime. Trust > the ATTR_CTIME flag in the update and only update the ctime when it's > set. > > Tested-by: Randy Dunlap # documentation bits > Signed-off-by: Jeff Layton Looks good. Feel free to add: Reviewed-by: Jan Kara Honza > --- > fs/attr.c | 28 +++++++++++++-------- > fs/inode.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/fs.h | 2 ++ > 3 files changed, 92 insertions(+), 10 deletions(-) > > diff --git a/fs/attr.c b/fs/attr.c > index 3bcbc45708a3..392eb62aa609 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -286,16 +286,20 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr) > unsigned int ia_valid = attr->ia_valid; > struct timespec64 now; > > - /* > - * If the ctime isn't being updated then nothing else should be > - * either. > - */ > - if (!(ia_valid & ATTR_CTIME)) { > - WARN_ON_ONCE(ia_valid & (ATTR_ATIME|ATTR_MTIME)); > - return; > + if (ia_valid & ATTR_CTIME) { > + /* > + * In the case of an update for a write delegation, we must respect > + * the value in ia_ctime and not use the current time. > + */ > + if (ia_valid & ATTR_DELEG) > + now = inode_set_ctime_deleg(inode, attr->ia_ctime); > + else > + now = inode_set_ctime_current(inode); > + } else { > + /* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */ > + WARN_ON_ONCE(ia_valid & ATTR_MTIME); > } > > - now = inode_set_ctime_current(inode); > if (ia_valid & ATTR_ATIME_SET) > inode_set_atime_to_ts(inode, attr->ia_atime); > else if (ia_valid & ATTR_ATIME) > @@ -354,8 +358,12 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode, > inode_set_atime_to_ts(inode, attr->ia_atime); > if (ia_valid & ATTR_MTIME) > inode_set_mtime_to_ts(inode, attr->ia_mtime); > - if (ia_valid & ATTR_CTIME) > - inode_set_ctime_to_ts(inode, attr->ia_ctime); > + if (ia_valid & ATTR_CTIME) { > + if (ia_valid & ATTR_DELEG) > + inode_set_ctime_deleg(inode, attr->ia_ctime); > + else > + inode_set_ctime_to_ts(inode, attr->ia_ctime); > + } > } > EXPORT_SYMBOL(setattr_copy); > > diff --git a/fs/inode.c b/fs/inode.c > index 4ec1e71e9a9d..7a324d999816 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -2751,6 +2751,78 @@ struct timespec64 inode_set_ctime_current(struct inode *inode) > } > EXPORT_SYMBOL(inode_set_ctime_current); > > +/** > + * inode_set_ctime_deleg - try to update the ctime on a delegated inode > + * @inode: inode to update > + * @update: timespec64 to set the ctime > + * > + * Attempt to atomically update the ctime on behalf of a delegation holder. > + * > + * The nfs server can call back the holder of a delegation to get updated > + * inode attributes, including the mtime. When updating the mtime we may > + * need to update the ctime to a value at least equal to that. > + * > + * This can race with concurrent updates to the inode, in which > + * case we just don't do the update. > + * > + * Note that this works even when multigrain timestamps are not enabled, > + * so use it in either case. > + */ > +struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update) > +{ > + struct timespec64 now, cur_ts; > + u32 cur, old; > + > + /* pairs with try_cmpxchg below */ > + cur = smp_load_acquire(&inode->i_ctime_nsec); > + cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED; > + cur_ts.tv_sec = inode->i_ctime_sec; > + > + /* If the update is older than the existing value, skip it. */ > + if (timespec64_compare(&update, &cur_ts) <= 0) > + return cur_ts; > + > + ktime_get_coarse_real_ts64_mg(&now); > + > + /* Clamp the update to "now" if it's in the future */ > + if (timespec64_compare(&update, &now) > 0) > + update = now; > + > + update = timestamp_truncate(update, inode); > + > + /* No need to update if the values are already the same */ > + if (timespec64_equal(&update, &cur_ts)) > + return cur_ts; > + > + /* > + * Try to swap the nsec value into place. If it fails, that means > + * we raced with an update due to a write or similar activity. That > + * stamp takes precedence, so just skip the update. > + */ > +retry: > + old = cur; > + if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) { > + inode->i_ctime_sec = update.tv_sec; > + mgtime_counter_inc(mg_ctime_swaps); > + return update; > + } > + > + /* > + * Was the change due to someone marking the old ctime QUERIED? > + * If so then retry the swap. This can only happen once since > + * the only way to clear I_CTIME_QUERIED is to stamp the inode > + * with a new ctime. > + */ > + if (!(old & I_CTIME_QUERIED) && (cur == (old | I_CTIME_QUERIED))) > + goto retry; > + > + /* Otherwise, it was a new timestamp. */ > + cur_ts.tv_sec = inode->i_ctime_sec; > + cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED; > + return cur_ts; > +} > +EXPORT_SYMBOL(inode_set_ctime_deleg); > + > /** > * in_group_or_capable - check whether caller is CAP_FSETID privileged > * @idmap: idmap of the mount @inode was found from > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 23908bad166c..b1a3bd07711b 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1584,6 +1584,8 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb, > > struct timespec64 current_time(struct inode *inode); > struct timespec64 inode_set_ctime_current(struct inode *inode); > +struct timespec64 inode_set_ctime_deleg(struct inode *inode, > + struct timespec64 update); > > static inline time64_t inode_get_atime_sec(const struct inode *inode) > { > > -- > 2.46.2 > -- Jan Kara SUSE Labs, CR