On Fri, 2024-10-25 at 13:31 +0000, Trond Myklebust wrote: > On Fri, 2024-10-25 at 12:52 +0000, Chuck Lever III wrote: > > > > > > > On Oct 24, 2024, at 11:00 PM, NeilBrown <neilb@xxxxxxx> 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. > > No, and after discussing it with others here at the bake-a-thon, I think that might be the best path forward here. At a high level: Add a callback to the client that runs just before calling nfsd_file_cache_purge() in expkey_flush(). The client would be expected to return all of its nfsd_files "soon" and switch back to normal network I/O. After that point, it can try to get a new nfsd_file and start up localio at that point. With that change too you can switch to using non-GC'ed nfsd_files. The client can just hold them open and do I/O to them at will. I think that's probably less brittle than trying to keep opaque inode pointers around, and would probably mean better performance since you won't be thrashing the filecache as much. -- Jeff Layton <jlayton@xxxxxxxxxx>
