On Mon 23-06-25 14:22:26, Amir Goldstein wrote:
> On Mon, Jun 23, 2025 at 1:58 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >
> > On Mon, Jun 23, 2025 at 01:55:38PM +0200, Jan Kara wrote:
> > > On Mon 23-06-25 11:01:28, Christian Brauner wrote:
> > > > Introduce new pidfs file handle values.
> > > >
> > > > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> > > > ---
> > > > include/linux/exportfs.h | 11 +++++++++++
> > > > 1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > > > index 25c4a5afbd44..45b38a29643f 100644
> > > > --- a/include/linux/exportfs.h
> > > > +++ b/include/linux/exportfs.h
> > > > @@ -99,6 +99,11 @@ enum fid_type {
> > > > */
> > > > FILEID_FAT_WITH_PARENT = 0x72,
> > > >
> > > > + /*
> > > > + * 64 bit inode number.
> > > > + */
> > > > + FILEID_INO64 = 0x80,
> > > > +
> > > > /*
> > > > * 64 bit inode number, 32 bit generation number.
> > > > */
> > > > @@ -131,6 +136,12 @@ enum fid_type {
> > > > * Filesystems must not use 0xff file ID.
> > > > */
> > > > FILEID_INVALID = 0xff,
> > > > +
> > > > + /* Internal kernel fid types */
> > > > +
> > > > + /* pidfs fid types */
> > > > + FILEID_PIDFS_FSTYPE = 0x100,
> > > > + FILEID_PIDFS = FILEID_PIDFS_FSTYPE | FILEID_INO64,
> > >
> > > What is the point behind having FILEID_INO64 and FILEID_PIDFS separately?
> > > Why not just allocate one value for FILEID_PIDFS and be done with it? Do
> > > you expect some future extensions for pidfs?
> >
> > I wouldn't rule it out, yes. This was also one of Amir's suggestions.
>
> The idea was to parcel the autonomous fid type to fstype (pidfs)
> which determines which is the fs to decode the autonomous fid
> and a per-fs sub-type like we have today.
>
> Maybe it is a bit over design, but I don't think this is really limiting us
> going forward, because those constants are not part of the uapi.
OK, I agree these file handles do not survive reboot anyway so we are free
to redefine the encoding in the future. So it is not a big deal (but it
also wouldn't be a big deal to start simple and add some subtyping in the
future when there's actual usecase). But in the current patch set we have
one flag FILEID_IS_AUTONOMOUS which does provide this subtyping and then
this FILEID_PIDFS_FSTYPE which doesn't seem to be about subtyping but about
pidfs expecting some future extensions and wanting to recognize all its
file handle types more easily (without having to enumerate all types like
other filesystems)? My concern is that fh_type space isn't that big and if
every filesystem started to reserve flag-like bits in it, we'd soon run out
of it. So I don't think this is a great precedens although in this
particular case I agree it can be modified in the future if we decide so...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
