Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 16 Aug 2022 08:51:58 -0700
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Jeff Layton <jlayton@kernel.org>
Cc:     David Howells <dhowells@redhat.com>,
        Christian Brauner <brauner@kernel.org>,
        viro@zeniv.linux.org.uk, linux-afs@lists.infradead.org,
        linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
        ceph-devel@vger.kernel.org
Subject: Re: [PATCH 1/4] vfs: report change attribute in statx for
 IS_I_VERSION inodes
Message-ID: <Yvu9HsCgzwpEYhPc@magnolia>
References: <ef692314ada01fd2117b730ef0afae50102974f5.camel@kernel.org>
 <20220816134419.xra4krb3jwlm4npk@wittgenstein>
 <20220816132759.43248-1-jlayton@kernel.org>
 <20220816132759.43248-2-jlayton@kernel.org>
 <4066396.1660658141@warthog.procyon.org.uk>
 <12637.1660662903@warthog.procyon.org.uk>
 <83d07cc4f7fe2ca9976d3f418e5137f354e933a4.camel@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <83d07cc4f7fe2ca9976d3f418e5137f354e933a4.camel@kernel.org>
Precedence: bulk

On Tue, Aug 16, 2022 at 11:32:24AM -0400, Jeff Layton wrote:
> On Tue, 2022-08-16 at 16:15 +0100, David Howells wrote:
> > Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > > I think we'll just have to ensure that before we expose this for any
> > > filesystem that it conforms to some minimum standards. i.e.: it must
> > > change if there are data or metadata changes to the inode, modulo atime
> > > changes due to reads on regular files or readdir on dirs.
> > > 
> > > The local filesystems, ceph and NFS should all be fine. I guess that
> > > just leaves AFS. If it can't guarantee that, then we might want to avoid
> > > exposing the counter for it.
> > 
> > AFS monotonically increments the counter on data changes; doesn't make any
> > change for metadata changes (other than the file size).
> > 
> > But you can't assume NFS works as per your suggestion as you don't know what's
> > backing it (it could be AFS, for example - there's a converter for that).
> > 
> 
> In that case, the NFS server must synthesize a proper change attr. The
> NFS spec mandates that it change on most metadata changes.
> 
> > Further, for ordinary disk filesystems, two data changes may get elided and
> > only increment the counter once.
> > 
> 
> Not a problem as long as nothing queried the counter in between the
> changes.
> 
> > And then there's mmap...
> > 
> 
> Not sure how that matters here.
> 
> > It might be better to reduce the scope of your definition and just say that it
> > must change if there's a data change and may also be changed if there's a
> > metadata change.
> > 
> 
> I'd prefer that we mandate that it change on metadata changes as well.

...in that case, why not leave the i_version bump in
xfs_trans_log_inode?  That will capture all changes to file data,
attribues, and metadata. ;)

--D

> That's what most of the in-kernel users want, and what most of the
> existing filesystems provide. If AFS can't give that guarantee then we
> can just omit exposing i_version on it.
> -- 
> Jeff Layton <jlayton@kernel.org>