Re: cache fixes (redux)

[Eric Van Hensbergen <ericvh@kernel.org>]

On Fri, Dec 29, 2023 at 09:50:39PM +0900, asmadeus@codewreck.org wrote:
> 
> It's not strictly needed though; in practice the vfs can handle
> duplicate i_ino as long as appropriate functions are used for iget
> (iget5_locked in fs/9p/vfs_inode.c); although now I'm thinking about it
> that won't work for programs like tar that'll notice the inode number is
> identical and consider the second file to be a hard link of the first...
>

This is the problem though, iget functions use lookup functions which
right now compare a bunch of state requiring a getattr to validate.  This
is one of the things I was trying to eliminate.  The other option would
be putting some of these additional bits (device, i_generation, etc.) in 
the qid.

>
> I guess we could swipe it under the rug by making the inode number a
> hash of everything in qid.path, and pretend collisions never happenTM?
> 

feels wrong, hashes always have collisions and it makes me uncomfortable.
I know some of the other on disk file systems have moved to 128-bit inodes,
does that help or just push out the problem (or make it less likely I suppose).
The combo of qid.path and qid.version is probably a pretty good mix, if
qid.version is some hash on time then the likelyhood of collision goes way
down (but notably isn't zero..) and servers don't always populate qid.version,
but that's something to fix server side.
 
> 
> ... Actually didn't think it'd be this easy to reproduce, but ext4
> reliably recyles the same inodes after unlink:
> h
> # touch a b c
> # ls -li a b c
> 357 -rw-r--r--. 1 root root 0 Dec 29 21:39 a
> 365 -rw-r--r--. 1 root root 0 Dec 29 21:39 b
> 366 -rw-r--r--. 1 root root 0 Dec 29 21:39 c
> # rm -f a b c
> # touch new
> # ls -li new
> 357 -rw-r--r--. 1 root root 0 Dec 29 21:40 new
> # rm -f new
> # touch again
> # ls -li again
> 357 -rw-r--r--. 1 root root 0 Dec 29 21:40 again
> 
> (works even if I actually write something into the files and sync them)
>
...
> 
> If the file's still open, it's not removed from the fs -- we're
> guaranted the inode won't come back yet.
> For a networked file system though I was thinking of cache more than
> open files, e.g. in the previous example a 9p client might think 'new'
> and 'again' are the same file.

So this is just a cache race problem potentially.  The real concern 
being that the cache is out of date with the server and then we get
an inode collision.  But that probably likely for other reasons with
a loose cache -- I guess the question is how to handle for a tight
cache.  qid.path+qid.vers maybe enough - but races still possible.

>
> I guess it doesn't really matter since we have no cache invalidation /
> refresh logic...
>

Well, we have logic - its just always valid or always invalid.  The real
question is how do we want it to work in tight mode.  Is qid valid or do
we always need to do a stat/getattr.

    -eric