Date:   Wed, 9 Aug 2023 17:00:41 +0200
From:   Jan Kara <jack@...e.cz>
To:     OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Cc:     Jeff Layton <jlayton@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Eric Van Hensbergen <ericvh@...nel.org>,
        Latchesar Ionkov <lucho@...kov.net>,
        Dominique Martinet <asmadeus@...ewreck.org>,
        Christian Schoenebeck <linux_oss@...debyte.com>,
        David Howells <dhowells@...hat.com>,
        Marc Dionne <marc.dionne@...istor.com>,
        Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, Xiubo Li <xiubli@...hat.com>,
        Ilya Dryomov <idryomov@...il.com>,
        Jan Harkes <jaharkes@...cmu.edu>, coda@...cmu.edu,
        Tyler Hicks <code@...icks.com>, Gao Xiang <xiang@...nel.org>,
        Chao Yu <chao@...nel.org>,
        Yue Hu <huyue2@...jj8bn.sched.sma.tdnsstic1.cn>,
        Jeffle Xu <jefflexu@...ux.alibaba.com>,
        Namjae Jeon <linkinjeon@...nel.org>,
        Sungjong Seo <sj1557.seo@...sung.com>,
        Jan Kara <jack@...e.com>, Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Miklos Szeredi <miklos@...redi.hu>,
        Bob Peterson <rpeterso@...hat.com>,
        Andreas Gruenbacher <agruenba@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna@...nel.org>,
        Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        Mark Fasheh <mark@...heh.com>,
        Joel Becker <jlbec@...lplan.org>,
        Joseph Qi <joseph.qi@...ux.alibaba.com>,
        Mike Marshall <hubcap@...ibond.com>,
        Martin Brandenburg <martin@...ibond.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Iurii Zaikin <yzaikin@...gle.com>,
        Steve French <sfrench@...ba.org>,
        Paulo Alcantara <pc@...guebit.com>,
        Ronnie Sahlberg <ronniesahlberg@...il.com>,
        Shyam Prasad N <sprasad@...rosoft.com>,
        Tom Talpey <tom@...pey.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Richard Weinberger <richard@....at>,
        Hans de Goede <hdegoede@...hat.com>,
        Hugh Dickins <hughd@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Amir Goldstein <amir73il@...il.com>,
        "Darrick J. Wong" <djwong@...nel.org>,
        Benjamin Coddington <bcodding@...hat.com>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        v9fs@...ts.linux.dev, linux-afs@...ts.infradead.org,
        linux-btrfs@...r.kernel.org, ceph-devel@...r.kernel.org,
        codalist@...emann.coda.cs.cmu.edu, ecryptfs@...r.kernel.org,
        linux-erofs@...ts.ozlabs.org, linux-ext4@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net, cluster-devel@...hat.com,
        linux-nfs@...r.kernel.org, ntfs3@...ts.linux.dev,
        ocfs2-devel@...ts.linux.dev, devel@...ts.orangefs.org,
        linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org,
        linux-mtd@...ts.infradead.org, linux-mm@...ck.org,
        linux-unionfs@...r.kernel.org, linux-xfs@...r.kernel.org
Subject: Re: [PATCH v7 05/13] fat: make fat_update_time get its own timestamp

On Wed 09-08-23 22:36:29, OGAWA Hirofumi wrote:
> Jeff Layton <jlayton@...nel.org> writes:
> 
> > On Wed, 2023-08-09 at 17:37 +0900, OGAWA Hirofumi wrote:
> >> Jeff Layton <jlayton@...nel.org> writes:
> >> 
> >> > Also, it may be that things have changed by the time we get to calling
> >> > fat_update_time after checking inode_needs_update_time. Ensure that we
> >> > attempt the i_version bump if any of the S_* flags besides S_ATIME are
> >> > set.
> >> 
> >> I'm not sure what it meaning though, this is from
> >> generic_update_time(). Are you going to change generic_update_time()
> >> too? If so, it doesn't break lazytime feature?
> >> 
> >
> > Yes. generic_update_time is also being changed in a similar fashion.
> > This shouldn't break the lazytime feature: lazytime is all about how and
> > when timestamps get written to disk. This work is all about which
> > clocksource the timestamps originally come from.
> 
> I can only find the following update in this series, another series
> updates generic_update_time()? The patch updates only if S_VERSION is
> set.
> 
> Your fat patch sets I_DIRTY_SYNC always instead of I_DIRTY_TIME. When I
> last time checked lazytime, and it was depending on I_DIRTY_TIME.
> 
> Are you sure it doesn't break lazytime? I'm totally confusing, and
> really similar with generic_update_time()?

Since you are talking past one another with Jeff let me chime in here :). I
think you are worried about this hunk:

-	if ((flags & S_VERSION) && inode_maybe_inc_iversion(inode, false))
+	if ((flags & (S_VERSION|S_CTIME|S_MTIME)) && inode_maybe_inc_iversion(inode, false))
 		dirty_flags |= I_DIRTY_SYNC;

which makes the 'flags' test pass even if we just modified ctime or mtime.
But do note the second part of the if - inode_maybe_inc_iversion() - so we
are going to mark the inode dirty with I_DIRTY_SYNC only if someone queried
iversion since the last time we have incremented it.

So this hunk is not really changing how inode is marked dirty, it only
changes how often we check whether iversion needs increment and that should
be fine (and desirable). Hence lazytime isn't really broken by this in any
way.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists