On Mon, May 31, 2021 at 5:11 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Sat, 29 May 2021 at 18:05, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Wed, Sep 23, 2020 at 2:12 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Sep 23, 2020 at 11:57 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > > >
> > > > On Wed, Sep 23, 2020 at 10:44 AM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Wed, Sep 23, 2020 at 4:49 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > > > >
> > > > > > I think that the proper was to implement reliable persistent file
> > > > > > handles in fuse/virtiofs would be to add ENCODE/DECODE to
> > > > > > FUSE protocol and allow the server to handle this.
> > > > >
> > > > > Max Reitz (Cc-d) is currently looking into this.
> > > > >
> > > > > One proposal was to add LOOKUP_HANDLE operation that is similar to
> > > > > LOOKUP except it takes a {variable length handle, name} as input and
> > > > > returns a variable length handle *and* a u64 node_id that can be used
> > > > > normally for all other operations.
> > > > >
> >
> > Miklos, Max,
> >
> > Any updates on LOOKUP_HANDLE work?
> >
> > > > > The advantage of such a scheme for virtio-fs (and possibly other fuse
> > > > > based fs) would be that userspace need not keep a refcounted object
> > > > > around until the kernel sends a FORGET, but can prune its node ID
> > > > > based cache at any time. If that happens and a request from the
> > > > > client (kernel) comes in with a stale node ID, the server will return
> > > > > -ESTALE and the client can ask for a new node ID with a special
> > > > > lookup_handle(fh, NULL).
> > > > >
> > > > > Disadvantages being:
> > > > >
> > > > > - cost of generating a file handle on all lookups
> > > >
> > > > I never ran into a local fs implementation where this was expensive.
> > > >
> > > > > - cost of storing file handle in kernel icache
> > > > >
> > > > > I don't think either of those are problematic in the virtiofs case.
> > > > > The cost of having to keep fds open while the client has them in its
> > > > > cache is much higher.
> > > > >
> > > >
> > > > Sounds good.
> > > > I suppose flock() does need to keep the open fd on server.
> > >
> > > Open files are a separate issue and do need an active object in the server.
> > >
> > > The issue this solves is synchronizing "released" and "evicted"
> > > states of objects between server and client. I.e. when a file is
> > > closed (and no more open files exist referencing the same object) the
> > > dentry refcount goes to zero but it remains in the cache. In this
> > > state the server could really evict it's own cached object, but can't
> > > because the client can gain an active reference at any time via
> > > cached path lookup.
> > >
> > > One other solution would be for the server to send a notification
> > > (NOTIFY_EVICT) that would try to clean out the object from the server
> > > cache and respond with a FORGET if successful. But I sort of like
> > > the file handle one better, since it solves multiple problems.
> > >
> >
> > Even with LOOKUP_HANDLE, I am struggling to understand how we
> > intend to invalidate all fuse dentries referring to ino X in case the server
> > replies with reused ino X with a different generation that the one stored
> > in fuse inode cache.
> >
> > This is an issue that I encountered when running the passthrough_hp test,
> > on my filesystem. In tst_readdir_big() for example, underlying files are being
> > unlinked and new files created reusing the old inode numbers.
> >
> > This creates a situation where server gets a lookup request
> > for file B that uses the reused inode number X, while old file A is
> > still in fuse dentry cache using the older generation of real inode
> > number X which is still in fuse inode cache.
> >
> > Now the server knows that the real inode has been rused, because
> > the server caches the old generation value, but it cannot reply to
> > the lookup request before the old fuse inode has been invalidated.
> > IIUC, fuse_lowlevel_notify_inval_inode() is not enough(?).
> > We would also need to change fuse_dentry_revalidate() to
> > detect the case of reused/invalidated inode.
> >
> > The straightforward way I can think of is to store inode generation
> > in fuse_dentry. It won't even grow the size of the struct.
> >
> > Am I over complicating this?
>
> In this scheme the generation number is already embedded in the file
> handle. If LOOKUP_HANDLE returns a nodeid that can be found in the
> icache, but which doesn't match the new file handle, then the old
> inode will be marked bad and a new one allocated.
>
> Does that answer your worries? Or am I missing something?
It affirms my understanding of the future implementation, but
does not help my implementation without protocol changes.
I thought I could get away without LOOKUP_HANDLE for
underlying fs that is able to resolve by ino, but seems that I still have an
unhandled corner case, so will need to add some kernel patch.
Unless there is already a way to signal from server to make the
inode bad in a synchronous manner (I did not find any) before
replying to LOOKUP with a new generation of the same ino.
Any idea about the timeline for LOOKUP_HANDLE?
I may be able to pick this up myself if there is no one actively
working on it or plans for anyone to make this happen.
Thanks,
Amir.
