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;
Date:   Thu, 23 Jun 2022 10:38:22 +1000
From:   Dave Chinner <david@fromorbit.com>
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs@vger.kernel.org, netdev@vger.kernel.org, tgraf@suug.ch,
        jlayton@redhat.com
Subject: Re: [PATCH RFC 29/30] NFSD: Convert the filecache to use rhashtable
Message-ID: <20220623003822.GF1098723@dread.disaster.area>
References: <165590626293.75778.9843437418112335153.stgit@manet.1015granger.net>
 <165590735674.75778.2489188434203366753.stgit@manet.1015granger.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <165590735674.75778.2489188434203366753.stgit@manet.1015granger.net>
Precedence: bulk

On Wed, Jun 22, 2022 at 10:15:56AM -0400, Chuck Lever wrote:
> Enable the filecache hash table to start small, then grow with the
> workload. Smaller server deployments benefit because there should
> be lower memory utilization. Larger server deployments should see
> improved scaling with the number of open files.
> 
> I know this is a big and messy patch, but there's no good way to
> rip out and replace a data structure like this.
> 
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Pretty sure I mentioned converting to rhashtable as well when we
were talking about the pointer-chasing overhead of list and tree
based indexing of large caches.  :)

> +
> +/*
> + * Atomically insert a new nfsd_file item into nfsd_file_rhash_tbl.
> + *
> + * Return values:
> + *   %NULL: @new was inserted successfully
> + *   %A valid pointer: @new was not inserted, a matching item is returned
> + *   %ERR_PTR: an unexpected error occurred during insertion
> + */
> +static struct nfsd_file *nfsd_file_insert(struct nfsd_file *new)
> +{
> +	struct nfsd_file_lookup_key key = {
> +		.type	= NFSD_FILE_KEY_FULL,
> +		.inode	= new->nf_inode,
> +		.need	= new->nf_flags,
> +		.net	= new->nf_net,
> +		.cred	= current_cred(),
> +	};
> +	struct nfsd_file *nf;
> +
> +	nf = rhashtable_lookup_get_insert_key(&nfsd_file_rhash_tbl,
> +					      &key, &new->nf_rhash,
> +					      nfsd_file_rhash_params);
> +	if (!nf)
> +		return nf;

The insert can return an error (e.g. -ENOMEM) so need to check
IS_ERR(nf) here as well.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com