Re: [PATCH v8 3/6] liveupdate: add LUO_SESSION_MAGIC magic inode type

[Christian Brauner <brauner@kernel.org>]

On Mon, Apr 20, 2026 at 03:22:03PM +0100, Luca Boccassi wrote:
> On Mon, 20 Apr 2026 at 13:26, Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Sat, Apr 18, 2026 at 05:28:20PM +0100, luca.boccassi@gmail.com wrote:
> > > +}
> > > +
> > > +static const struct dentry_operations luo_session_dentry_operations = {
> > > +     .d_dname        = luo_session_dname,
> > > +};
> > > +
> > > +static int luo_session_init_fs_context(struct fs_context *fc)
> > > +{
> > > +     struct pseudo_fs_context *ctx;
> > > +
> > > +     ctx = init_pseudo(fc, LUO_SESSION_MAGIC);
> >
> > I'd just call that LUO_FS_MAGIC.
> 
> It's specific for the session fd, so I'd rather keep it as-is. Pasha
> what do you think?
> 
> > > +     return 0;
> > > +}
> > > +
> > > +static struct file_system_type luo_session_fs_type = {
> > > +     .name = "luo_session",
> > > +     .init_fs_context = luo_session_init_fs_context,
> > > +     .kill_sb = kill_anon_super,
> > > +};
> > > +
> > >  /* Create a "struct file" for session */
> > >  static int luo_session_getfile(struct luo_session *session, struct file **filep)
> >
> > Luo is going full anti-pattern here. This whole return via a function
> > argument completely messes up the later codepths. We don't do manual
> > get_unused_fd_flags() flags and then file in new code, and then fail
> > in-between:
> 
> This is getting a bit too complex for me and it's all existing stuff,
> so I'll leave it for Pasha and the other maintainers to look at later,
> they are CC'ed
> 
> > >  {
> > > -     char name_buf[128];
> > > +     char name_buf[LIVEUPDATE_SESSION_NAME_LENGTH + 1];
> > >       struct file *file;
> > >
> > >       lockdep_assert_held(&session->mutex);
> > > -     snprintf(name_buf, sizeof(name_buf), "[luo_session] %s", session->name);
> > > -     file = anon_inode_getfile(name_buf, &luo_session_fops, session, O_RDWR);
> > > -     if (IS_ERR(file))
> > > +
> > > +     ihold(luo_session_inode);
> >
> > Right, you're now sharing the same inode among all luo sessions. So
> > you've gained the ability to recognize luo inodes via fstatfs() but you
> > still can't compare two luo session file descriptors for equality using
> > stat() which is a major win and if you're doing this work anyway, let's
> > just go the extra step.
> 
> I'm not aware of any use case for this, it wasn't even discussed, so
> I'll leave this out for now, if the need comes up it can certainly be
> revisited later. A singleton is nice as it saves some resources.

I've almost written the code for you in my first reply and it's also a
literal 1:1 templated copy from what I already did pidfs as well.

Let's do things right from the start and not taky nasty shortcuts to fix
one minor urge without having any foresight at all. If you want to have
an actual tiny filesystem to identify an inode type do it properly and
add the semantics to allocate separate inodes and get all the goodies at
once and make it possible to maintain this properly.

You will need this for so many other reasons than inode comparison such
as lsm integration which definitely want to end up managing this. None
of this works with a single inode. This is not difficult.

And this cop-out would never fly in systemd. I'm regularly sent through
15 review rounds to implement 5 different approaches to the same
problem. So are you. Please take the code I've given you and simply copy
and paste it over what you already did and solve this correctly once and
for all instead of hacking up a filesystem for a single inode so you can
define an inode type.