Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 19 Apr 2021 15:53:15 -0400
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Bruce Fields <bfields@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 2/5] nfsd: hash nfs4_files by inode number
Message-ID: <20210419195315.GA17661@fieldses.org>
References: <1618596018-9899-1-git-send-email-bfields@redhat.com>
 <1618596018-9899-2-git-send-email-bfields@redhat.com>
 <7342A4CD-0B93-4C13-AD32-4DADC26CD8ED@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7342A4CD-0B93-4C13-AD32-4DADC26CD8ED@oracle.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
From:   bfields@fieldses.org (J. Bruce Fields)
Precedence: bulk

On Fri, Apr 16, 2021 at 07:21:16PM +0000, Chuck Lever III wrote:
> 
> 
> > On Apr 16, 2021, at 2:00 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > The nfs4_file structure is per-filehandle, not per-inode, because the
> > spec requires open and other state to be per filehandle.
> > 
> > But it will turn out to be convenient for nfs4_files associated with the
> > same inode to be hashed to the same bucket, so let's hash on the inode
> > instead of the filehandle.
> > 
> > Filehandle aliasing is rare, so that shouldn't have much performance
> > impact.
> > 
> > (If you have a ton of exported filesystems, though, and all of them have
> > a root with inode number 2, could that get you an overlong has chain?
> 
> ^has ^hash
> 
> Also, I'm getting this new warning:
> 
> /home/cel/src/linux/linux/include/linux/hash.h:81:38: warning: shift too big (4294967104) for type unsigned long long

Whoops; it needs this: would you like me to resend?--b.

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b0c74dbde07b..47a76284b47c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -547,7 +547,7 @@ static unsigned int file_hashval(struct svc_fh *fh)
 	struct inode *inode = d_inode(fh->fh_dentry);
 
 	/* XXX: why not (here & in file cache) use inode? */
-	return (unsigned int)hash_long(inode->i_ino, FILE_HASH_SIZE);
+	return (unsigned int)hash_long(inode->i_ino, FILE_HASH_BITS);
 }
 
 static struct hlist_head file_hashtbl[FILE_HASH_SIZE];