Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Sep 2022 08:11:06 -0400
From:   "Theodore Ts'o" <tytso@mit.edu>
To:     Jeff Layton <jlayton@kernel.org>
Cc:     "J. Bruce Fields" <bfields@fieldses.org>, Jan Kara <jack@suse.cz>,
        NeilBrown <neilb@suse.de>, adilger.kernel@dilger.ca,
        djwong@kernel.org, david@fromorbit.com, trondmy@hammerspace.com,
        viro@zeniv.linux.org.uk, zohar@linux.ibm.com, xiubli@redhat.com,
        chuck.lever@oracle.com, lczerner@redhat.com, brauner@kernel.org,
        fweimer@redhat.com, linux-man@vger.kernel.org,
        linux-api@vger.kernel.org, linux-btrfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org,
        linux-nfs@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [man-pages RFC PATCH v4] statx, inode: document the new
 STATX_INO_VERSION field
Message-ID: <YxstWiu34TfJ6muW@mit.edu>
References: <79aaf122743a295ddab9525d9847ac767a3942aa.camel@kernel.org>
 <20220907125211.GB17729@fieldses.org>
 <771650a814ab1ff4dc5473d679936b747d9b6cf5.camel@kernel.org>
 <20220907135153.qvgibskeuz427abw@quack3>
 <166259786233.30452.5417306132987966849@noble.neil.brown.name>
 <20220908083326.3xsanzk7hy3ff4qs@quack3>
 <YxoIjV50xXKiLdL9@mit.edu>
 <02928a8c5718590bea5739b13d6b6ebe66cac577.camel@kernel.org>
 <20220908155605.GD8951@fieldses.org>
 <9e06c506fd6b3e3118da0ec24276e85ea3ee45a1.camel@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9e06c506fd6b3e3118da0ec24276e85ea3ee45a1.camel@kernel.org>
Precedence: bulk

On Thu, Sep 08, 2022 at 01:40:11PM -0400, Jeff Layton wrote:
> 
> Ted, how would we access this? Maybe we could just add a new (generic)
> super_block field for this that ext4 (and other filesystems) could
> populate at mount time?

Yeah, I was thinking about just adding it to struct super, with some
value (perhaps 0 or ~0) meaning that the file system didn't support
it.  If people were concerned about struct super bloat, we could also
add some new function to struct super_ops that would return one or
more values that are used rarely by most of the kernel code, and so
doesn't need to be in the struct super data structure.  I don't have
strong feelings one way or another.

On another note, my personal opinion is that at least as far as ext4
is concerned, i_version on disk's only use is for NFS's convenience,
and so I have absolutely no problem with changing how and when
i_version gets updated modulo concerns about impacting performance.
That's one of the reasons why being able to update i_version only
lazily, so that if we had, say, some workload that was doing O_DIRECT
writes followed by fdatasync(), there wouldn't be any obligation to
flush the inode out to disk just because we had bumped i_version
appeals to me.

But aside from that, I don't consider when i_version gets updated on
disk, especially what the semantics are after a crash, and if we need
to change things so that NFS can be more performant, I'm happy to
accomodate.  One of the reasons why we implemented the ext4 fast
commit feature was to improve performance for NFS workloads.

I know some XFS developers have some concerns here, but I just wanted
to make it be explicit that (a) I'm not aware of any users who are
depending on the i_version on-disk semantics, and (b) if they are
depending on something which as far as I'm concerned in an internal
implementation detail, we've made no promises to them, and they can
get to keep both pieces.  :-)  This is especially since up until now,
there is no supported, portable userspace interface to make i_version
available to userspace.

Cheers,

					- Ted