Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 08:32:55AM +0200, Amir Goldstein wrote:
> On Tue, Dec 12, 2023 at 7:53 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Tue, Dec 12, 2023 at 11:59:51AM +1100, NeilBrown wrote:
> > > On Tue, 12 Dec 2023, Kent Overstreet wrote:
> > > > On Tue, Dec 12, 2023 at 10:53:07AM +1100, NeilBrown wrote:
> > > > > On Tue, 12 Dec 2023, Kent Overstreet wrote:
> > > > > > On Tue, Dec 12, 2023 at 09:43:27AM +1100, NeilBrown wrote:
> > > > > > > On Sat, 09 Dec 2023, Kent Overstreet wrote:
> > > > > > Thoughts?
> > > > > >
> > > > >
> > > > > I'm completely in favour of exporting the (full) filehandle through
> > > > > statx. (If the application asked for the filehandle, it will expect a
> > > > > larger structure to be returned.  We don't need to use the currently
> > > > > reserved space).
> > > > >
> > > > > I'm completely in favour of updating user-space tools to use the
> > > > > filehandle to check if two handles are for the same file.
> > > > >
> > > > > I'm not in favour of any filesystem depending on this for correct
> > > > > functionality today.  As long as the filesystem isn't so large that
> > > > > inum+volnum simply cannot fit in 64 bits, we should make a reasonable
> > > > > effort to present them both in 64 bits.  Depending on the filehandle is a
> > > > > good plan for long term growth, not for basic functionality today.
> > > >
> > > > My standing policy in these situations is that I'll do the stopgap/hacky
> > > > measure... but not before doing actual, real work on the longterm
> > > > solution :)
> > >
> > > Eminently sensible.
> > >
> > > >
> > > > So if we're all in favor of statx as the real long term solution, how
> > > > about we see how far we get with that?
> > > >
> > >
> > > I suggest:
> > >
> > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > >                               same inode number

This is just ugly with questionable value. A constant reminder of how
broken this is. Exposing the subvolume id also makes this somewhat redundant.

> > >
> > >
> > >  __u64 stx_vol     Volume identifier.  Two files with same stx_vol and
> > >                    stx_ino MUST be the same.  Exact meaning of volumes
> > >                    is filesys-specific

Exposing this makes sense. I've mentioned that I'd be open to exporting
the subvolume id in statx before even though I know some people don't
like that. I still stand by that. Especially now that we have btrfs and
bcachefs that both have the concept of a subvolume id it makes sense to
put it into statx().

> > >
> > >  STATX_VOL         Want stx_vol
> > >
> > >   __u8 stx_handle_len  Length of stx_handle if present
> > >   __u8 stx_handle[128] Unique stable identifier for this file.  Will
> > >                        NEVER be reused for a different file.
> > >                        This appears AFTER __statx_pad2, beyond
> > >                        the current 'struct statx'.
> > >  STATX_HANDLE      Want stx_handle_len and stx_handle. Buffer for
> > >                    receiving statx info has at least
> > >                    sizeof(struct statx)+128 bytes.

No, we're should not be reinventing another opaque handle. And even if
it really doesn't belong into statx().

> >
> > Hmmm.
> >
> > Doesn't anyone else see or hear the elephant trumpeting loudly in
> > the middle of the room?
> >
> > I mean, we already have name_to_handle_at() for userspace to get a
> > unique, opaque, filesystem defined file handle for any given file.

I agree.

> > It's the same filehandle that filesystems hand to the nfsd so nfs
> > clients can uniquely identify the file they are asking the nfsd to
> > operate on.
> >
> > The contents of these filehandles is entirely defined by the file
> > system and completely opaque to the user. The only thing that
> > parses the internal contents of the handle is the filesystem itself.
> > Therefore, as long as the fs encodes the information it needs into the
> > handle to determine what subvol/snapshot the inode belongs to when
> > the handle is passed back to it (e.g. from open_by_handle_at()) then
> > nothing else needs to care how it is encoded.
> >
> > So can someone please explain to me why we need to try to re-invent
> > a generic filehandle concept in statx when we already have a
> > have working and widely supported user API that provides exactly
> > this functionality?
> 
> Yeh.
> 
> Not to mention that since commit 64343119d7b8 ("exportfs: support encoding
> non-decodeable file handles by default"), exporting file handles as strong
> object identifiers is not limited to filesystems that support NFS export.
> 
> All fs have a default implementation of encode_fh() by encoding a file id
> of type FILEID_INO64_GEN from { i_ino, i_generation } and any fs can
> define its own encode_fh() operation (e.g. to include subvol id) even without
> implementing the decode fh operations needed for NFS export.

I also would like both btrfs and bcachefs to agree.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[David Howells]

Christian Brauner <brauner@kernel.org> wrote:

> > > > I suggest:
> > > >
> > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > >                               same inode number
> 
> This is just ugly with questionable value. A constant reminder of how
> broken this is. Exposing the subvolume id also makes this somewhat redundant.

There is a upcoming potential problem where even the 64-bit field I placed in
statx() may be insufficient.  The Auristor AFS server, for example, has a
96-bit vnode ID, but I can't properly represent this in stx_ino.  Currently, I
just truncate the value to fit and hope that the discarded part will be all
zero, but that's not really a good thing to do - especially when stx_ino is
used programmatically to check for hardlinks.

Would it be better to add an 'stx_ino_2' field and corresponding flag?

David

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 09:10:47AM +0000, David Howells wrote:
> Christian Brauner <brauner@kernel.org> wrote:
> 
> > > > > I suggest:
> > > > >
> > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > >                               same inode number
> > 
> > This is just ugly with questionable value. A constant reminder of how
> > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> 
> There is a upcoming potential problem where even the 64-bit field I placed in
> statx() may be insufficient.  The Auristor AFS server, for example, has a
> 96-bit vnode ID, but I can't properly represent this in stx_ino.  Currently, I

Is that vnode ID akin to a volume? Because if so you could just
piggy-back on a subvolume id field in statx() and expose it there.

> just truncate the value to fit and hope that the discarded part will be all
> zero, but that's not really a good thing to do - especially when stx_ino is
> used programmatically to check for hardlinks.
> 
> Would it be better to add an 'stx_ino_2' field and corresponding flag?

Would this be meaningfully different from using a file handle?

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 10:23, Christian Brauner <brauner@kernel.org> wrote:
>
> On Tue, Dec 12, 2023 at 09:10:47AM +0000, David Howells wrote:
> > Christian Brauner <brauner@kernel.org> wrote:
> >
> > > > > > I suggest:
> > > > > >
> > > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > > >                               same inode number
> > >
> > > This is just ugly with questionable value. A constant reminder of how
> > > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> >
> > There is a upcoming potential problem where even the 64-bit field I placed in
> > statx() may be insufficient.  The Auristor AFS server, for example, has a
> > 96-bit vnode ID, but I can't properly represent this in stx_ino.  Currently, I
>
> Is that vnode ID akin to a volume? Because if so you could just
> piggy-back on a subvolume id field in statx() and expose it there.

And how would exporting a subvolume ID and expecting userspace to take
that into account when checking for hard links be meaningfully
different than expecting userspace to retrieve the file handle and
compare that?

The latter would at least be a generic solution, including stacking fs
inodes, instead of tackling just a specific corner of the problem
space.

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 10:28:12AM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 10:23, Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Tue, Dec 12, 2023 at 09:10:47AM +0000, David Howells wrote:
> > > Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > > > > > I suggest:
> > > > > > >
> > > > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > > > >                               same inode number
> > > >
> > > > This is just ugly with questionable value. A constant reminder of how
> > > > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> > >
> > > There is a upcoming potential problem where even the 64-bit field I placed in
> > > statx() may be insufficient.  The Auristor AFS server, for example, has a
> > > 96-bit vnode ID, but I can't properly represent this in stx_ino.  Currently, I
> >
> > Is that vnode ID akin to a volume? Because if so you could just
> > piggy-back on a subvolume id field in statx() and expose it there.
> 
> And how would exporting a subvolume ID and expecting userspace to take
> that into account when checking for hard links be meaningfully
> different than expecting userspace to retrieve the file handle and
> compare that?
> 
> The latter would at least be a generic solution, including stacking fs
> inodes, instead of tackling just a specific corner of the problem
> space.

So taking a step back here, please. The original motivation for this
discussion was restricted to handle btrfs - and now bcachefs as well.
Both have a concept of a subvolume so it made sense to go that route.
IOW, it wasn't originally a generic problem or pitched as such.

Would overlayfs be able to utilize an extended inode field as well?

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 10:35, Christian Brauner <brauner@kernel.org> wrote:

> So taking a step back here, please. The original motivation for this
> discussion was restricted to handle btrfs - and now bcachefs as well.
> Both have a concept of a subvolume so it made sense to go that route.
> IOW, it wasn't originally a generic problem or pitched as such.
>
> Would overlayfs be able to utilize an extended inode field as well?

Well, yes, but I don't think that's the right solution.

I think the right solution is to move to using file handles instead of
st_ino, the problem with that is that there's no way kernel can force
upgrading userspace.

It might help to have the fh in statx, since that's easier on the
userspace programmer than having to deal with two interfaces (i_ino
won't go away for some time, because of backward compatibility).
OTOH I also don't like the way it would need to be shoehorned into
statx.

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[David Howells]

Christian Brauner <brauner@kernel.org> wrote:

> > There is a upcoming potential problem where even the 64-bit field I placed
> > in statx() may be insufficient.  The Auristor AFS server, for example, has
> > a 96-bit vnode ID, but I can't properly represent this in stx_ino.
> > Currently, I
> 
> Is that vnode ID akin to a volume? Because if so you could just
> piggy-back on a subvolume id field in statx() and expose it there.

No.  The volume ID is the ID of the volume.  The vnode is the equivalent of an
inode.

> > just truncate the value to fit and hope that the discarded part will be all
> > zero, but that's not really a good thing to do - especially when stx_ino is
> > used programmatically to check for hardlinks.
> > 
> > Would it be better to add an 'stx_ino_2' field and corresponding flag?
> 
> Would this be meaningfully different from using a file handle?

There's also the matter of presenting the "inode number" to the user - "ls -i"
for example.

David

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 10:42:58AM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 10:35, Christian Brauner <brauner@kernel.org> wrote:
> 
> > So taking a step back here, please. The original motivation for this
> > discussion was restricted to handle btrfs - and now bcachefs as well.
> > Both have a concept of a subvolume so it made sense to go that route.
> > IOW, it wasn't originally a generic problem or pitched as such.
> >
> > Would overlayfs be able to utilize an extended inode field as well?
> 
> Well, yes, but I don't think that's the right solution.

Exposing the subvolume id in statx() is still fine imho. It's a concept
shared between btrfs and bcachefs and it's pretty useful for interested
userspace that wants to make use of these apis.

> 
> I think the right solution is to move to using file handles instead of
> st_ino, the problem with that is that there's no way kernel can force
> upgrading userspace.

That's not our job tbh. I get why this is desirable. What we can do is
advertise better and newer apis but we don't have to make unpleasant
compromises in our interfaces for that.

> 
> It might help to have the fh in statx, since that's easier on the
> userspace programmer than having to deal with two interfaces (i_ino
> won't go away for some time, because of backward compatibility).
> OTOH I also don't like the way it would need to be shoehorned into
> statx.

No, it really doesn't belong into statx().

And besides, the file handle apis name_to_handle_at() are already
in wider use than a lot of people think. Not just for the exportfs case
but also for example, cgroupfs uses file handles to provide unique
identifiers for cgroups that can be compared.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 14:48, Christian Brauner <brauner@kernel.org> wrote:

> Exposing the subvolume id in statx() is still fine imho. It's a concept
> shared between btrfs and bcachefs and it's pretty useful for interested
> userspace that wants to make use of these apis.

Exposing subvolume ID should be okay, as long as it's not advertised
as a way to uniquely identify an inode.   Its use should be limited to
finding subvolume boundaries.

> > It might help to have the fh in statx, since that's easier on the
> > userspace programmer than having to deal with two interfaces (i_ino
> > won't go away for some time, because of backward compatibility).
> > OTOH I also don't like the way it would need to be shoehorned into
> > statx.
>
> No, it really doesn't belong into statx().
>
> And besides, the file handle apis name_to_handle_at() are already
> in wider use than a lot of people think. Not just for the exportfs case
> but also for example, cgroupfs uses file handles to provide unique
> identifiers for cgroups that can be compared.

The issue with name_to_handle_at() is its use of the old, non-unique
mount ID.  Yes, yes, we can get away with

 fd = openat(dfd, path, O_PATH);
 name_to_handle_at(fd, "", ..., AT_EMPTY_PATH);
 statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, ...);
 close(fd);

But that's *four* syscalls instead of one...

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 09:56:45AM +0100, Christian Brauner wrote:
> On Tue, Dec 12, 2023 at 08:32:55AM +0200, Amir Goldstein wrote:
> > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > >                               same inode number
> 
> This is just ugly with questionable value. A constant reminder of how
> broken this is. Exposing the subvolume id also makes this somewhat redundant.

Oh hell no. We finally get a reasonably productive discussion and I wake
up to you calling things ugly and broken, with no reason or
justification or any response to the justification that's already been
given?

Christain, that's not how we do things. Let's not turn what was a
productive discussion into a trainwreck, OK?

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 03:06:07PM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 14:48, Christian Brauner <brauner@kernel.org> wrote:
> 
> > Exposing the subvolume id in statx() is still fine imho. It's a concept
> > shared between btrfs and bcachefs and it's pretty useful for interested
> > userspace that wants to make use of these apis.
> 
> Exposing subvolume ID should be okay, as long as it's not advertised
> as a way to uniquely identify an inode.   Its use should be limited to
> finding subvolume boundaries.
> 
> > > It might help to have the fh in statx, since that's easier on the
> > > userspace programmer than having to deal with two interfaces (i_ino
> > > won't go away for some time, because of backward compatibility).
> > > OTOH I also don't like the way it would need to be shoehorned into
> > > statx.
> >
> > No, it really doesn't belong into statx().
> >
> > And besides, the file handle apis name_to_handle_at() are already
> > in wider use than a lot of people think. Not just for the exportfs case
> > but also for example, cgroupfs uses file handles to provide unique
> > identifiers for cgroups that can be compared.
> 
> The issue with name_to_handle_at() is its use of the old, non-unique
> mount ID.  Yes, yes, we can get away with
> 
>  fd = openat(dfd, path, O_PATH);
>  name_to_handle_at(fd, "", ..., AT_EMPTY_PATH);
>  statx(fd, "", AT_EMPTY_PATH, STATX_MNT_ID_UNIQUE, ...);
>  close(fd);
> 
> But that's *four* syscalls instead of one...

Yeah, but putting this into statx() isn't really nice imho. Once we do
actually land the unique mount id thing it wouldn't be the worst thing
to add name_to_handle_at2().

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 10:35:40AM +0100, Christian Brauner wrote:
> On Tue, Dec 12, 2023 at 10:28:12AM +0100, Miklos Szeredi wrote:
> > On Tue, 12 Dec 2023 at 10:23, Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > On Tue, Dec 12, 2023 at 09:10:47AM +0000, David Howells wrote:
> > > > Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > > > > > I suggest:
> > > > > > > >
> > > > > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > > > > >                               same inode number
> > > > >
> > > > > This is just ugly with questionable value. A constant reminder of how
> > > > > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> > > >
> > > > There is a upcoming potential problem where even the 64-bit field I placed in
> > > > statx() may be insufficient.  The Auristor AFS server, for example, has a
> > > > 96-bit vnode ID, but I can't properly represent this in stx_ino.  Currently, I
> > >
> > > Is that vnode ID akin to a volume? Because if so you could just
> > > piggy-back on a subvolume id field in statx() and expose it there.
> > 
> > And how would exporting a subvolume ID and expecting userspace to take
> > that into account when checking for hard links be meaningfully
> > different than expecting userspace to retrieve the file handle and
> > compare that?
> > 
> > The latter would at least be a generic solution, including stacking fs
> > inodes, instead of tackling just a specific corner of the problem
> > space.
> 
> So taking a step back here, please. The original motivation for this
> discussion was restricted to handle btrfs - and now bcachefs as well.
> Both have a concept of a subvolume so it made sense to go that route.
> IOW, it wasn't originally a generic problem or pitched as such.
> 
> Would overlayfs be able to utilize an extended inode field as well?

No, the original motivation was not just btrfs and bcachefs; overlayfs
fundamentally needs to export a bigger identifier than the host
filesystems - pigeonhole principle, if anyone remembers their
combinatorics.

This applies to any filesystem which is under the hood reexporting from
other filesystems; there's a lot of stuff going on in container
filesystems right now, and I expect it'll come up there (you can create
new identifiers if you're exporting file by file, but not if it's
directory trees).

And Neal brought up NFS re-exports; I think there's a compelling
argument to be made that we ought to be able to round trip NFS
filehandles.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Tue, Dec 12, 2023 at 10:16:31AM -0500, Kent Overstreet wrote:
> On Tue, Dec 12, 2023 at 09:56:45AM +0100, Christian Brauner wrote:
> > On Tue, Dec 12, 2023 at 08:32:55AM +0200, Amir Goldstein wrote:
> > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > >                               same inode number
> > 
> > This is just ugly with questionable value. A constant reminder of how
> > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> 
> Oh hell no. We finally get a reasonably productive discussion and I wake
> up to you calling things ugly and broken, with no reason or
> justification or any response to the justification that's already been
> given?
> 
> Christain, that's not how we do things. Let's not turn what was a
> productive discussion into a trainwreck, OK?

That's a major aggressive tone for no good reason. And probably not a
good way to convince anyone. I'm also not sure why you're taking a
dislike of this flag as a personal affront.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 04:29:09PM +0100, Christian Brauner wrote:
> On Tue, Dec 12, 2023 at 10:16:31AM -0500, Kent Overstreet wrote:
> > On Tue, Dec 12, 2023 at 09:56:45AM +0100, Christian Brauner wrote:
> > > On Tue, Dec 12, 2023 at 08:32:55AM +0200, Amir Goldstein wrote:
> > > > > >  STATX_ATTR_INUM_NOT_UNIQUE - it is possible that two files have the
> > > > > >                               same inode number
> > > 
> > > This is just ugly with questionable value. A constant reminder of how
> > > broken this is. Exposing the subvolume id also makes this somewhat redundant.
> > 
> > Oh hell no. We finally get a reasonably productive discussion and I wake
> > up to you calling things ugly and broken, with no reason or
> > justification or any response to the justification that's already been
> > given?
> > 
> > Christain, that's not how we do things. Let's not turn what was a
> > productive discussion into a trainwreck, OK?
> 
> That's a major aggressive tone for no good reason. And probably not a
> good way to convince anyone. I'm also not sure why you're taking a
> dislike of this flag as a personal affront.

No, if you're going to show up with comments like that it is entirely
called for.

Other poeple have been finding ways to contribute to the technical
discussion; just calling things "ugly and broken" does not.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:

> Other poeple have been finding ways to contribute to the technical
> discussion; just calling things "ugly and broken" does not.

Kent, calm down please.  We call things "ugly and broken" all the
time.  That's just an opinion, you are free to argue it, and no need
to take it personally.

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 04:38:29PM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> 
> > Other poeple have been finding ways to contribute to the technical
> > discussion; just calling things "ugly and broken" does not.
> 
> Kent, calm down please.  We call things "ugly and broken" all the
> time.  That's just an opinion, you are free to argue it, and no need
> to take it personally.

It's an entirely subjective judgement that has no place in a discussion
where we're trying to decide things on technical merits.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 16:43, Kent Overstreet <kent.overstreet@linux.dev> wrote:
>
> On Tue, Dec 12, 2023 at 04:38:29PM +0100, Miklos Szeredi wrote:
> > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> >
> > > Other poeple have been finding ways to contribute to the technical
> > > discussion; just calling things "ugly and broken" does not.
> >
> > Kent, calm down please.  We call things "ugly and broken" all the
> > time.  That's just an opinion, you are free to argue it, and no need
> > to take it personally.
>
> It's an entirely subjective judgement that has no place in a discussion
> where we're trying to decide things on technical merits.

Software is like architecture.  It must not collapse, but to function
well it also needs to look good.  That's especially relevant to APIs,
and yes, it's a matter of taste and can be very subjective.

On this particular point (STATX_ATTR_INUM_NOT_UNIQUE) I can completely
understand Christian's reaction.  But if you think this would be a
useful feature, please state your technical argument.

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 04:57:41PM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 16:43, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> >
> > On Tue, Dec 12, 2023 at 04:38:29PM +0100, Miklos Szeredi wrote:
> > > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > >
> > > > Other poeple have been finding ways to contribute to the technical
> > > > discussion; just calling things "ugly and broken" does not.
> > >
> > > Kent, calm down please.  We call things "ugly and broken" all the
> > > time.  That's just an opinion, you are free to argue it, and no need
> > > to take it personally.
> >
> > It's an entirely subjective judgement that has no place in a discussion
> > where we're trying to decide things on technical merits.
> 
> Software is like architecture.  It must not collapse, but to function
> well it also needs to look good.  That's especially relevant to APIs,
> and yes, it's a matter of taste and can be very subjective.

Good taste is a highly important trait - in the course of normal work we
trust our subjective opinions constantly, because those opinions have
been trained by years and years of judgements and we can't reason
through everything.

But when you show up to a discussion that's been going on for a page,
where everything's been constructively gathering input, and you start
namecalling - and crucially, _without giving any technical justification
for your opinions_ - that's just you being a dick.

> On this particular point (STATX_ATTR_INUM_NOT_UNIQUE) I can completely
> understand Christian's reaction.  But if you think this would be a
> useful feature, please state your technical argument.

Well, you could have scanned through the prior thread, or read the
extensive LWN coverage, but in short - inode number uniqueness is
something that's already impossible to guarantee, today.

In short, STATX_ATTR_INUM_NOT_UNIQUE is required to tell userspace when
they _must_ do the new thing if they care about correctness; it provides
a way to tell userspace what guarantees we're able to provide.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Miklos Szeredi]

On Tue, 12 Dec 2023 at 17:08, Kent Overstreet <kent.overstreet@linux.dev> wrote:

> In short, STATX_ATTR_INUM_NOT_UNIQUE is required to tell userspace when
> they _must_ do the new thing if they care about correctness; it provides
> a way to tell userspace what guarantees we're able to provide.

That flag would not help with improving userspace software.

What would help, if said software started using a unique identifier.
We already seem to have a unique ID in the form of file handles,
though some exotic filesystems might allow more than one fh to refer
to the same inode, so this still needs some looking into.

The big problem is that we can't do a lot about existing software, and
must keep trying to keep st_ino unique for the foreseeable future.

Thanks,
Miklos

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Tue, Dec 12, 2023 at 05:30:23PM +0100, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 17:08, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> 
> > In short, STATX_ATTR_INUM_NOT_UNIQUE is required to tell userspace when
> > they _must_ do the new thing if they care about correctness; it provides
> > a way to tell userspace what guarantees we're able to provide.
> 
> That flag would not help with improving userspace software.

I disagree. Just having it there and in the documentation will be a good
solid nudge to userspace programmers that this is something they need to
consider.

And: if we size this thing for NFSv4 filehandles, those are big: there's
an overhead associated with using those, since userspace generally has
to track in memory all inode numbers/file handles that it's seen.
STATX_ATTR_INUM_NOT_UNIQUE would allow userspace to avoid that overhead
when it is safe to do so.

(But remember that file handles include inode generation numbers, and
inode numbers do not; that is something we should remember to document,
and explain why it is important).

> What would help, if said software started using a unique identifier.
> We already seem to have a unique ID in the form of file handles,
> though some exotic filesystems might allow more than one fh to refer
> to the same inode, so this still needs some looking into.
> 
> The big problem is that we can't do a lot about existing software, and
> must keep trying to keep st_ino unique for the foreseeable future.

Whatever hacks we try to apply to st_ino are outside the scope of this
discussion. Right now, we need to be figuring out what our future
proofed interface is going to be, so that we don't end up kicking this
can down the road when st_ino will be _really_ space constrained.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[NeilBrown]

On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> 
> > Other poeple have been finding ways to contribute to the technical
> > discussion; just calling things "ugly and broken" does not.
> 
> Kent, calm down please.  We call things "ugly and broken" all the
> time.  That's just an opinion, you are free to argue it, and no need
> to take it personally.

But maybe we shouldn't.  Maybe we should focus on saying what, exactly,
is unpleasant to look at and way.  Or what exactly causes poor
funcationality.
"ugly" and "broken" are not particularly useful words in a technical
discussion.  I understand people want to use them, but they really need
to be backed up with details.  It is details that matter.

NeilBrown


> 
> Thanks,
> Miklos
> 
> 

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[NeilBrown]

On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> On Tue, 12 Dec 2023 at 17:08, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> 
> > In short, STATX_ATTR_INUM_NOT_UNIQUE is required to tell userspace when
> > they _must_ do the new thing if they care about correctness; it provides
> > a way to tell userspace what guarantees we're able to provide.
> 
> That flag would not help with improving userspace software.

Are you sure?

Suppose I wanted to export an filesystem using some protocol (maybe one
called "NFSv4"), and suppose this protocol supported the communication
of an attribute called "fileid" which was optional but requires to be
fully unique if provided at all.

If I had access to STATX_ATTR_INUM_NOT_UNIQUE, then I could not export
the fileid when it didn't met the protocol requirements, but could when
it did.

This may not be a strong case for the inclusion of the flag it is, I
think, a clear indication that "would not help" is what our fact
checkers would call "over-reach".

NeilBrown

> 
> What would help, if said software started using a unique identifier.
> We already seem to have a unique ID in the form of file handles,
> though some exotic filesystems might allow more than one fh to refer
> to the same inode, so this still needs some looking into.
> 
> The big problem is that we can't do a lot about existing software, and
> must keep trying to keep st_ino unique for the foreseeable future.
> 
> Thanks,
> Miklos
> 

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

> But when you show up to a discussion that's been going on for a page,

On the bcachefs mailing list without fsdevel or anyone else Cced.

> where everything's been constructively gathering input, and you start
> namecalling - and crucially, _without giving any technical justification

I didn't namecall at all. I just didn't like the flag and called it
"ugly" and explicitly said that I didn't see the value it brings. And
I'm not the only one.

I've been pretty supportive of the other parts of this. So I truly don't
understand why your turning this into a personal thing.

> for your opinions_ - that's just you being a dick.

Calling me a dick even if just implied is clearly crossing a line.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Wed, Dec 13, 2023 at 08:46:54AM +1100, NeilBrown wrote:
> On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > 
> > > Other poeple have been finding ways to contribute to the technical
> > > discussion; just calling things "ugly and broken" does not.
> > 
> > Kent, calm down please.  We call things "ugly and broken" all the
> > time.  That's just an opinion, you are free to argue it, and no need
> > to take it personally.
> 
> But maybe we shouldn't.  Maybe we should focus on saying what, exactly,
> is unpleasant to look at and way.  Or what exactly causes poor
> funcationality.

I said it's "ugly" and I doubted it's value. I didn't call it "broken".
And I've been supportive of the other parts. Yet everyone seems fine
with having this spiral out of control to the point where I'm being
called a dick.

You hade a privat discussion on the bcachefs mailing list and it seems
you expected to show up here with a complete interface that we just all
pick up and merge even though this is a multi-year longstanding
argument.

I've been supportive of both the subvol addition to statx and the
STATX_* flag to indicate a subvolume root. Yet somehow you're all
extremely focussed on me disliking this flag.

> "ugly" and "broken" are not particularly useful words in a technical
> discussion.  I understand people want to use them, but they really need
> to be backed up with details.  It is details that matter.

I did say that I don't see the value. And it's perfectly ok for you to
reiterate why it provides value. Your whole discussion has been private
on some other mailing list without the relevant maintainers Cced.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Christian Brauner]

On Wed, Dec 13, 2023 at 10:48:01AM +0100, Christian Brauner wrote:
> On Wed, Dec 13, 2023 at 08:46:54AM +1100, NeilBrown wrote:
> > On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> > > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > > 
> > > > Other poeple have been finding ways to contribute to the technical
> > > > discussion; just calling things "ugly and broken" does not.
> > > 
> > > Kent, calm down please.  We call things "ugly and broken" all the
> > > time.  That's just an opinion, you are free to argue it, and no need
> > > to take it personally.
> > 
> > But maybe we shouldn't.  Maybe we should focus on saying what, exactly,
> > is unpleasant to look at and way.  Or what exactly causes poor
> > funcationality.
> 
> I said it's "ugly" and I doubted it's value. I didn't call it "broken".

I see where you took that from. To be clear, what I meant by broken is
the device number switching that btrfs has been doing which has caused
so much pain already and is at least partially responsible for this
endless long discussion. I didn't mean "broken" as in the flag is
broken. I acknowledge that I failed to make that clearer.

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[NeilBrown]

On Wed, 13 Dec 2023, Christian Brauner wrote:
> On Wed, Dec 13, 2023 at 08:46:54AM +1100, NeilBrown wrote:
> > On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> > > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > > 
> > > > Other poeple have been finding ways to contribute to the technical
> > > > discussion; just calling things "ugly and broken" does not.
> > > 
> > > Kent, calm down please.  We call things "ugly and broken" all the
> > > time.  That's just an opinion, you are free to argue it, and no need
> > > to take it personally.
> > 
> > But maybe we shouldn't.  Maybe we should focus on saying what, exactly,
> > is unpleasant to look at and way.  Or what exactly causes poor
> > funcationality.
> 
> I said it's "ugly" and I doubted it's value. I didn't call it "broken".
> And I've been supportive of the other parts. Yet everyone seems fine
> with having this spiral out of control to the point where I'm being
> called a dick.
> 
> You hade a privat discussion on the bcachefs mailing list and it seems
> you expected to show up here with a complete interface that we just all
> pick up and merge even though this is a multi-year longstanding
> argument.

I thought I was still having that private discussion on the bcachefs
mailing list.  I didn't realise that fsdevel had been added.

NeilBrown


> 
> I've been supportive of both the subvol addition to statx and the
> STATX_* flag to indicate a subvolume root. Yet somehow you're all
> extremely focussed on me disliking this flag.
> 
> > "ugly" and "broken" are not particularly useful words in a technical
> > discussion.  I understand people want to use them, but they really need
> > to be backed up with details.  It is details that matter.
> 
> I did say that I don't see the value. And it's perfectly ok for you to
> reiterate why it provides value. Your whole discussion has been private
> on some other mailing list without the relevant maintainers Cced.
> 

Re: file handle in statx (was: Re: How to cope with subvolumes and snapshots on muti-user systems?)

[Kent Overstreet]

On Fri, Dec 15, 2023 at 09:47:47AM +1100, NeilBrown wrote:
> On Wed, 13 Dec 2023, Christian Brauner wrote:
> > On Wed, Dec 13, 2023 at 08:46:54AM +1100, NeilBrown wrote:
> > > On Wed, 13 Dec 2023, Miklos Szeredi wrote:
> > > > On Tue, 12 Dec 2023 at 16:35, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > > > 
> > > > > Other poeple have been finding ways to contribute to the technical
> > > > > discussion; just calling things "ugly and broken" does not.
> > > > 
> > > > Kent, calm down please.  We call things "ugly and broken" all the
> > > > time.  That's just an opinion, you are free to argue it, and no need
> > > > to take it personally.
> > > 
> > > But maybe we shouldn't.  Maybe we should focus on saying what, exactly,
> > > is unpleasant to look at and way.  Or what exactly causes poor
> > > funcationality.
> > 
> > I said it's "ugly" and I doubted it's value. I didn't call it "broken".
> > And I've been supportive of the other parts. Yet everyone seems fine
> > with having this spiral out of control to the point where I'm being
> > called a dick.
> > 
> > You hade a privat discussion on the bcachefs mailing list and it seems
> > you expected to show up here with a complete interface that we just all
> > pick up and merge even though this is a multi-year longstanding
> > argument.
> 
> I thought I was still having that private discussion on the bcachefs
> mailing list.  I didn't realise that fsdevel had been added.

I only thought that the discussion had gotten to the point where we had
something concrete enough to start to circulate more widely.

I wonder if some introspection could go into why this has been a
multi-year longstanding argument.