> On Oct 25, 2024, at 9:31 AM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> 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. We've discussed this privately. A notification callback is possible, but that's code that would have to be written, and using GC nfsd_files was an expedient for getting LOCALIO merged. There are some corner cases that will need to be worked through to help determine whether a callback is truly a simpler design. -- Chuck Lever
