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

[Dominique Martinet <asmadeus@codewreck.org>]

Sorry for the delay...

Tingmao Wang wrote on Sun, Apr 06, 2025 at 09:43:01PM +0100:
> Unrelated to the above problem, it also seems like even with the revert in
> [2], because in cached mode inode are still reused based on qid (and type,
> version (aka mtime), etc), the setup mentioned in [2] still causes
> problems in th latest kernel with cache=loose:

cache=loose is "you're on your own", I think it's fine to keep as is,
especially given qemu can handle it with multidevs=remap if required

> With the above in mind, I have a proposal for 9pfs to:
> 1. Reuse inodes even in uncached mode
> 2. However, reuse them based on qid.path AND the actual pathname, by doing
>    the appropriate testing in v9fs_test(_new)?_inode(_dotl)?

I think that's fine for cache=none, but it breaks hardlinks on
cache=loose so I think this ought to only be done without cache
(I haven't really played with the cache flag bits, not check pathname if
any of loose, writeback or metadata are set?)

> The main problem here is how to store the pathname in a sensible way and
> tie it to the inode.  For now I opted with an array of names acquired with
> take_dentry_name_snapshot, which reuses the same memory as the dcache to
> store the actual strings, but doesn't tie the lifetime of the dentry with
> the inode (I thought about holding a reference to the dentry in the
> v9fs_inode, but it seemed like a wrong approach and would cause dentries
> to not be evicted/released).

That's pretty hard to get right and I wish we had more robust testing
there... But I guess that's appropriate enough.

I know Atos has done an implementation that keeps the full path
somewhere to re-open fids in case of server reconnections, but that code
has never been submitted upstream that I can see so I can't check how
they used to store the path :/ Ohwell.

> Storing one pathname per inode also means we don't reuse the same inode
> for hardlinks -- maybe this can be fixed as well in a future version, if
> this approach sounds good?

Ah, you pointed it out yourself. I don't see how we could fix that, as
we have no way other than the qid to identify hard links; so this really
ought to depend on cache level if you want to support landlock/*notify
in cache=none.

Frankly the *notify use-case is difficult to support properly, as files
can change from under us (e.g. modifying the file directly on the host
in qemu case, or just multiple mounts of the same directory), so it
can't be relied on in the general case anyway -- 9p doesn't have
anything like NFSv4 leases to get notified when other clients write a
file we "own", so whatever we do will always be limited...
But I guess it can make sense for limited monitoring e.g. rebuilding
something on change and things like that?


-- 
Dominique Martinet | Asmadeus