Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever III" <chuck.lever@oracle.com>
Cc:     "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 7/9] NFSD: Use rhashtable for managing nfs4_file objects
In-reply-to: <E69F66C3-C17E-4397-8329-8B6C2D2E2F0F@oracle.com>
References: <166507275951.1802.13184584115155050247.stgit@manet.1015granger.net>,
 <166507324882.1802.884870684212914640.stgit@manet.1015granger.net>,
 <166544739751.14457.9018300177489236723@noble.neil.brown.name>,
 <EB08B095-BF02-4B5E-8CD2-12B0201328D2@oracle.com>,
 <166553144435.32740.14940127200777208215@noble.neil.brown.name>,
 <4004BFE1-C887-4A53-9512-8A264E0361FF@oracle.com>,
 <166560951668.32740.3528791072339550207@noble.neil.brown.name>,
 <E69F66C3-C17E-4397-8329-8B6C2D2E2F0F@oracle.com>
Date:   Fri, 14 Oct 2022 09:14:42 +1100
Message-id: <166569928288.32740.15383309396957908665@noble.neil.brown.name>
Precedence: bulk

On Fri, 14 Oct 2022, Chuck Lever III wrote:
> 
> > On Oct 12, 2022, at 5:18 PM, NeilBrown <neilb@suse.de> 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.

> 
> 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 see that there can be several nfsd_file per inode - in different
network namespaces, or with different credentials or different access
modes.  That really needs to be fixed.

Thanks,
NeilBrown