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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8DE1C88CB6 for ; Mon, 12 Jun 2023 13:52:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235916AbjFLNwE (ORCPT ); Mon, 12 Jun 2023 09:52:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40048 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236666AbjFLNwA (ORCPT ); Mon, 12 Jun 2023 09:52:00 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 91A64E6F; Mon, 12 Jun 2023 06:51:56 -0700 (PDT) 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 2E84C62976; Mon, 12 Jun 2023 13:51:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 92EC5C433A1; Mon, 12 Jun 2023 13:51:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1686577915; bh=N35u9HNcA2GE6O0RLNmO/H/bI7DFCPIb/J9WbCfp+7I=; h=Subject:From:To:Date:In-Reply-To:References:From; b=ObL7C8SEHs/LzGPrSxC74RrT9T2yAr7+1XglnX3EW1FZM90y7O6ormobkdpAWuZ0e RdLzx900Hqpy9HE3EfCtu+74BEGhE/aZtEqml+3NJgQSzQx53YAlTOFBgpsZKSbcTh +zXor8GIZ20+va1qDjHAfR3RyRUEaBOMhmjJ7l5g2lpz8/OpJX+h5I3T6nxDG8Q0Ct vPS6mDUmgWbpb1+t3cmEUqFflpPCwKrf9+eWZ+e4rjAuOfKobUeruOpGBYBZ90Wbpa C9ELvqhW19k3IAANYXAYjz0pZ8SMZZaeiSfoRUPDrDPMlZiF0tSo1r2eVMBMPNiBkq +CwHQ9soLqvPA== Message-ID: Subject: Re: [PATCH v2 8/8] cifs: update the ctime on a partial page write From: Jeff Layton To: Tom Talpey , Christian Brauner , Al Viro , Brad Warrum , Ritu Agarwal , Arnd Bergmann , Greg Kroah-Hartman , Ian Kent , "Tigran A. Aivazian" , Jeremy Kerr , Ard Biesheuvel , Namjae Jeon , Sungjong Seo , Steve French , Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , John Johansen , Paul Moore , James Morris , "Serge E. Hallyn" , Ruihan Li , Sebastian Reichel , Alan Stern , Suren Baghdasaryan , Wolfram Sang , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, autofs@vger.kernel.org, linux-efi@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-cifs@vger.kernel.org, samba-technical@lists.samba.org, apparmor@lists.ubuntu.com, linux-security-module@vger.kernel.org Date: Mon, 12 Jun 2023 09:51:50 -0400 In-Reply-To: References: <20230612104524.17058-1-jlayton@kernel.org> <20230612104524.17058-9-jlayton@kernel.org> 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 Precedence: bulk List-ID: On Mon, 2023-06-12 at 09:41 -0400, Tom Talpey wrote: > On 6/12/2023 6:45 AM, Jeff Layton wrote: > > POSIX says: > >=20 > > "Upon successful completion, where nbyte is greater than 0, write(= ) > > shall mark for update the last data modification and last file st= atus > > change timestamps of the file..." > >=20 > > Add the missing ctime update. > >=20 > > Signed-off-by: Jeff Layton > > --- > > fs/smb/client/file.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > > index df88b8c04d03..a00038a326cf 100644 > > --- a/fs/smb/client/file.c > > +++ b/fs/smb/client/file.c > > @@ -2596,7 +2596,7 @@ static int cifs_partialpagewrite(struct page *pag= e, unsigned from, unsigned to) > > write_data, to - from, &offset); > > cifsFileInfo_put(open_file); > > /* Does mm or vfs already set times? */ > > - inode->i_atime =3D inode->i_mtime =3D current_time(inode); > > + inode->i_atime =3D inode->i_mtime =3D inode->i_ctime =3D current_tim= e(inode); >=20 > Question. It appears that roughly half the filesystems in this series > don't touch the i_atime in this case. And the other half do. Which is > correct? Did they incorrectly set i_atime instead of i_ctime? >=20 I noticed that too, and with this set, I decided to not make any atime changes since I wasn't sure. I think the answer to your question is "it depends". atime is supposed to be updated on reads, not writes, but sometimes a write requires a RMW cycle of some flavor so one can imagine that in some cases we'd need to update all three. In this case, I'm not sure that updating any of these times is the right thing to do. This is called from ->launder_folio, so the syscall that issued the write is long gone and we're in writeback here. With NFS, we generally leave timestamp updates to the server. Should any of these timestamps be updated by the (SMB1) client here? --=20 Jeff Layton