Re: [PATCH v2 7/9] NFSD: Use rhashtable for managing nfs4_file objects

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> 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







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux