Re: [PATCH] nfsd/filecache: add nfsd_file_acquire_gc_cached

[Trond Myklebust <trondmy@hammerspace.com>]

On Fri, 2024-10-25 at 12:52 +0000, Chuck Lever III wrote:
> 
> 
> > On Oct 24, 2024, at 11:00 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > I'm wondering about the request for a garbage-collected nfsd_file
> > though.  For NFSv3 that makes sense.  For NFSv4 we would expect the
> > file
> > to already be open as a non-garbage-collected nfsd_file and opening
> > it
> > again seems wasteful.  That doesn't need to be fixed for this patch
> > and
> > maybe doesn't need to be fixed at all, but it seemed worth
> > highlighting.
> 
> I don't think using a GC'd nfsd_file for LOCALIO is a bug.
> 
> NFSv4 guarantees an OPEN to pin the nfsd_file, and a CLOSE
> (or lease expiry) to release it. There's no desire for or
> need for garbage collection.
> 
> NFSv3 and LOCALIO use each nfsd_file for the life of one I/O
> operation, and that nfsd_file is cached in the expectation
> that another I/O operation on the same file will happen with
> frequent temporarl proximity. Garbage collection is needed
> for both cases.
> 

No. There is a huge difference between the two. In the case of NFSv3,
the server has no idea whether or not there is further need for the
file (stateless), whereas in the localio case, the client is able to
tell exactly when the application holds the file open or not
(stateful).

The only reason for jumping through all these hoops in the case of
localio is the 'user may want to unmount' exception.

Is there any reason why we couldn't add a notification for that, to
give knfsd the opportunity to clear out any open file state? The
current approach appears to be flat out optimising for the exception.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com