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

[Tingmao Wang]

On 9/16/25 00:31, Dominique Martinet wrote:
> (Thanks Christian, replying just once but your reply was helpful)
> 
> Tingmao Wang wrote on Mon, Sep 15, 2025 at 02:44:44PM +0100:
>>> I'm fuzzy on the details but I don't see how inode pointers would be
>>> stable for other filesystems as well, what prevents
>>> e.g. vm.drop_caches=3 to drop these inodes on ext4?
>>
>> Landlock holds a reference to the inode in the ruleset, so they shouldn't
>> be dropped.  On security_sb_delete Landlock will iput those inodes so they
>> won't cause issue with unmounting.  There is some special mechanism
>> ("landlock objects") to decouple the ruleset themselves from the actual
>> inodes, so that previously Landlocked things can keep running even after
>> the inode has disappeared as a result of unmounting.
> 
> Thank you for the explanation, that makes more sense.
> 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.

> 
>> [...]
>>
>> I tried mounting a qemu-exported 9pfs backed on ext4, with
>> multidevs=remap, and created a file, used stat to note its inode number,
>> deleted the file, created another file (of the same OR different name),
>> and that new file will have the same inode number.
>>
>> (If I don't delete the file, then a newly created file would of course
>> have a different ext4 inode number, and in that case QEMU exposes a
>> different qid)
> 
> Ok so from Christian's reply this is just ext4 reusing the same inode..
> I briefly hinted at this above, but in this case ext4 will give the
> inode a different generation number (so the ext4 file handle will be
> different, and accessing the old one will get ESTALE); but that's not
> something qemu currently tracks and it'd be a bit of an overhaul...
> In theory qemu could hash mount_id + file handle to get a properly
> unique qid, if we need to improve that, but that'd be limited to root
> users (and to filesystems that support name_to_handle_at) so I don't
> think it's really appropriate either... hmm..

Actually I think I forgot that there is also qid.version, which in the
case of a QEMU-exported 9pfs might just be the file modification time?  In
9pfs currently we do reject a inode match if that version changed server
side in cached mode:

v9fs_test_inode_dotl:
	/* compare qid details */
	if (memcmp(&v9inode->qid.version,
		   &st->qid.version, sizeof(v9inode->qid.version)))
		return 0;

(not tested whether QEMU correctly sets this version yet)

> 
> (I also thought of checking if nlink is 0 when getting a new inode, but
> that's technically legimitate from /proc/x/fd opens so I don't think we
> can do that either)
> 
> And then there's also all the servers that don't give unique qids at
> all, so we'll just get weird landlock/fsnotify behaviours for them if we
> go that way...
> 
> -----------------
> 
> Okay, you've convinced me something like path tracking seems more
> appropriate; I'll just struggle one last time first with a few more open
> questions:
>  - afaiu this (reusing inodes) work in cached mode because the dentry is
> kept around;

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_vfs_lookup:
	...
	else if (v9ses->cache & (CACHE_META|CACHE_LOOSE))
		inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb);
	else
		inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);
	...
v9fs_qid_iget_dotl:
	...
	if (new)
		test = v9fs_test_new_inode_dotl;
	else
		test = v9fs_test_inode_dotl;
	...
v9fs_test_new_inode_dotl:
	static int v9fs_test_new_inode_dotl(struct inode *inode, void *data)
	{
		return 0;
	}


> I don't understand the vfs well enough but could the inodes
> hold its dentry and dentries hold their parent dentry alive somehow?
> 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.  However holding a proper dentry reference in an
inode might be tricky as dentry holds the reference to its inode.

> 
> 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.

> 
>  - 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.
> 
> Thanks!

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.

(This doesn't solve the problem for fsnotify though)

Kind regards,
Tingmao