Re: [RFC PATCH v7 9/9] vfs: expose STATX_VERSION to userland

Jeff Layton
2022-10-18T14:57:07Z

On Tue, 2022-10-18 at 06:35 -0400, Jeff Layton wrote:
> On Tue, 2022-10-18 at 09:14 +1100, Dave Chinner wrote:
> > On Mon, Oct 17, 2022 at 06:57:09AM -0400, Jeff Layton wrote:
> > > From: Jeff Layton <jlayton@redhat.com>
> > >
> > > Claim one of the spare fields in struct statx to hold a 64-bit inode
> > > version attribute. When userland requests STATX_VERSION, copy the
> > > value from the kstat struct there, and stop masking off
> > > STATX_ATTR_VERSION_MONOTONIC.
> >
> > Can we please make the name more sepcific than "version"? It's way
> > too generic and - we already have userspace facing "version" fields
> > for inodes that refer to the on-disk format version exposed in
> > various UAPIs. It's common for UAPI structures used for file
> > operations to have a "version" field that refers to the *UAPI
> > structure version* rather than file metadata or data being retrieved
> > from the file in question.
> >
> > The need for an explanatory comment like this:
> >
> > > + __u64 stx_version; /* Inode change attribute */
> >
> > demonstrates it is badly named. If you want it known as an inode
> > change attribute, then don't name the variable "version". In
> > reality, it really needs to be an opaque cookie, not something
> > applications need to decode directly to make sense of.
> >
>
> Fair enough. I started with this being named stx_change_attr and other
> people objected. I then changed to stx_ino_version, but the "_ino"
> seemed redundant.
>
> I'm open to suggestions here. Naming things like this is hard.
>

How about:

STATX_CHANGE / statx->stx_change / STATX_ATTR_CHANGE_MONOTONIC

?
--
Jeff Layton <jlayton@kernel.org>