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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>
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
Thread-Topic: [PATCH 2/5] nfsd: hash nfs4_files by inode number
Thread-Index: AQHXMupixIvrGePep02kzvU78UvPJ6q3hYIAgAS/7oCAAA24gA==
Date:   Mon, 19 Apr 2021 20:42:21 +0000
Message-ID: <0788B2C4-B2A5-4430-AE63-0E9DA82A7118@oracle.com>
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>
 <20210419195315.GA17661@fieldses.org>
In-Reply-To: <20210419195315.GA17661@fieldses.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <07D95CD1D670AF4085682E42CAC22B2D@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 214fd875-f005-40c2-7d87-08d90373a1c3
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Apr 2021 20:42:21.8875
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Dhq0V8MGiwXKajE4JQDNBB66G6fXG3jiRe43Lm/sVgQwjJWN07+sGokcryQxj0ItyEyzd0GRzTOrtj58W0N4rg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4494
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9959 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0
 mlxlogscore=999 malwarescore=0 adultscore=0 phishscore=0 bulkscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104060000 definitions=main-2104190142
X-Proofpoint-GUID: 5D_gIr7Bx19PGcmBAUf1KIXSrQALJj-X
X-Proofpoint-ORIG-GUID: 5D_gIr7Bx19PGcmBAUf1KIXSrQALJj-X
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9959 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 impostorscore=0 phishscore=0 mlxscore=0
 suspectscore=0 lowpriorityscore=0 mlxlogscore=999 spamscore=0 bulkscore=0
 adultscore=0 malwarescore=0 clxscore=1015 priorityscore=1501
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000
 definitions=main-2104190142
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


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

No, thanks. I'll squash this in.


> 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 =3D d_inode(fh->fh_dentry);
>=20
> 	/* 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);
> }
>=20
> static struct hlist_head file_hashtbl[FILE_HASH_SIZE];

--
Chuck Lever