Re: [PATCH v2 0/7] fs/9p: Reuse inode based on path (in addition to qid)

[Dominique Martinet <asmadeus@codewreck.org>]

Tingmao Wang wrote on Tue, Sep 16, 2025 at 01:44:27PM +0100:
> > iirc even in cacheless mode 9p should keep inode arounds if there's an
> > open fd somewhere
> 
> Yes, because there is a dentry that has a reference to it.  Similarly if
> there is a Landlock rule referencing it, the inode will also be kept
> around (but not the dentry, Landlock only references the inode).  The
> problem is that when another application (that is being Landlocked)
> accesses the file, 9pfs will create a new inode in uncached mode,
> regardless of whether an existing inode exists.
> [...]
> Based on my understanding, I think this isn't really to do with whether
> the dentry is around or not.  In cached mode, 9pfs will use iget5_locked
> to look up an existing inode based on the qid, if one exists, and use
> that, even if no cached dentry points to it.  However, in uncached mode,
> currently if vfs asks 9pfs to find an inode (e.g. because the dentry is no
> longer in cache), it always get a new one:
> [...]
> v9fs_qid_iget_dotl:
> 	...
> 	if (new)
> 		test = v9fs_test_new_inode_dotl;
> 	else
> 		test = v9fs_test_inode_dotl;

Right, if we get all the way to iget uncached mode will get a new inode,
but if the file is opened (I tried `cat > foo` and `tail -f foo`) then
re-opening cat will not issue a lookup at all -- v9fs_vfs_lookup() is
not called in the first place.
Likewise, in cached mode, just having the file in cache makes new open
not call v9fs_vfs_lookup for that file (even if it's not currently
open), so that `if (new)` is not actually what matters here afaiu.

What's the condition to make it happen? Can we make that happen with
landlock?

In practice that'd make landlock partially negate cacheless mode, as
we'd "pin" landlocked paths, but as long as readdirs aren't cached and
other metadata is refreshed on e.g. stat() calls I think that's fine
if we can make it happen.

(That's a big if)

> > So in cacheless mode, if you have a tree like this:
> > a
> > └── b
> >     ├── c
> >     └── d
> > with c 'open' (or a reference held by landlock), then dentries for a/b/c
> > would be kept, but d could be droppable?
> 
> I think, based on my understanding, a child dentry does always have a
> reference to its parent, and so parent won't be dropped before child, if
> child dentry is alive.

I'd be tempted to agree here

> However holding a proper dentry reference in an
> inode might be tricky as dentry holds the reference to its inode.

Hmm, yeah, that's problematic.
Could it be held in landlock and not by the inode?
Just thinking out loud.

> > My understanding is that in cacheless mode we're dropping dentries
> > aggressively so that things like readdir() are refreshed, but I'm
> > thinking this should work even if we keep some dentries alive when their
> > inode is held up.
> 
> If we have some way of keeping the dentry alive (without introducing
> circular reference problems) then I guess that would work and we don't
> have to track paths ourselves.

Yes, that's the idea - the dentry basically already contain the path, so
we wouldn't be reinventing the wheel...

> >  - if that doesn't work (or is too complicated), I'm thinking tracking
> > path is probably better than qid-based filtering based on what we
> > discussed as it only affects uncached mode.. I'll need to spend some
> > time testing but I think we can move forward with the current patchset
> > rather than try something new.
> 
> Note that in discussion with Mickaël (maintainer of Landlock) he indicated
> that he would be comfortable for Landlock to track a qid, instead of
> holding a inode, specifically for 9pfs.

Yes, I saw that, but what you pointed out about qid reuse make me
somewhat uncomfortable with that direction -- you could allow a
directory, delete it, create a new one somewhere else and if the
underlying fs reuse the same inode number the rule would allow an
intended directory instead so I'd rather not rely on qid for this
either.
But if you think that's not a problem in practice (because e.g. landlock
would somehow detect the dir got deleted or another good reason it's not
a problem) then I agree it's probably the simplest way forward
implementation-wise.

-- 
Dominique Martinet | Asmadeus