> On Oct 13, 2022, at 6:14 PM, NeilBrown <neilb@xxxxxxx> wrote: > > On Fri, 14 Oct 2022, Chuck Lever III wrote: >> >>> On Oct 12, 2022, at 5:18 PM, NeilBrown <neilb@xxxxxxx> wrote: >>> >>> On Thu, 13 Oct 2022, Chuck Lever III wrote: >>>> >>>> I think I stopped at the non-list variant of rhashtable because >>>> using rhl was more complex, and the non-list variant seemed to >>>> work fine. There's no architectural reason either file_hashtbl >>>> or the filecache must use the non-list variant. >>>> >>>> In any event, it's worth taking the trouble now to change the >>>> nfs4_file implementation proposed here as you suggest. >>> >>> If you like you could leave it as-is for now and I can provide a patch >>> to convert to rhl-tables later (won't be until late October). >>> There is one thing I would need to understand though: why are the >>> nfsd_files per-filehandle instead of per-inode? There is probably a >>> good reason, but I cannot think of one. >> >> I'm not clear on your offer: do you mean converting the nfsd_file >> cache from rhashtable to rhl, or converting the proposed nfs4_file >> rework? I had planned to do the latter myself and post a refresh. > > Either/both. Of course if you do the refresh, then I'll just review it. Yep, I plan to repost, as part of addressing (your) review comments. >> The nfsd_file_acquire API is the only place that seems to want a >> filehandle, and it's just to lookup the underlying inode. Perhaps >> I don't understand your question? > > Sorry, I meant nfs4_files, not nfsd_file: find_file() and find_or_add_file(). > Why is there one nfs4_file per filehandle I can't answer that (yet), but I suspect there is some semantic association between the [current] filehandle and a particular state ID that makes this a sensible arrangement. > I see that there can be several nfsd_file per inode - in different > network namespaces, or with different credentials or different access > modes. My impression is that is by design. Each namespace and unique credential needs a distinct nfsd_file. Each nfsd_file acts like an open file descriptor in user space. > That really needs to be fixed. I'm not sure I agree, but maybe "that" and "fixed" are doing some heavy lifting. Jeff might be able to add some insight on design purpose, and we can write a patch that adds that as a documenting comment somewhere. -- Chuck Lever
