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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Wang Yugui <wangyugui@e16-tech.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "david@fromorbit.com" <david@fromorbit.com>,
        "tgraf@suug.ch" <tgraf@suug.ch>, Jeff Layton <jlayton@redhat.com>
Subject: Re: [PATCH RFC 00/30] Overhaul NFSD filecache
Thread-Topic: [PATCH RFC 00/30] Overhaul NFSD filecache
Thread-Index: AQHYhkIqDsKitGhdzEmSu19Gpo96sa1bwYeAgAAHywA=
Date:   Wed, 22 Jun 2022 19:04:39 +0000
Message-ID: <FE520DC8-3C8F-4974-9F3B-84DE822CB899@oracle.com>
References: <165590626293.75778.9843437418112335153.stgit@manet.1015granger.net>
 <20220623023645.F914.409509F4@e16-tech.com>
In-Reply-To: <20220623023645.F914.409509F4@e16-tech.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?hjAscq+Cl/KkzVeg8X/cqNNWlg85tGqGxrG1s2WvniWXLCz45nmZEHbdeFeJ?=
 =?us-ascii?Q?j86jkjmcMhIhqCh7NyfBfm0FNXSn/B8vU32rTRZd96DhjmV1IS42t4/9GjCd?=
 =?us-ascii?Q?L5mPr/eSJcA93tX8rHlKaso8KKIyDnOA3IX+w6U0IqcfF6hHperNRDkHVN6z?=
 =?us-ascii?Q?pwt1qL9Ez77oUuxjF7HPrNckOr4bWKQ+fFWzDOipc69ThC4NpUv5r7GmxMTH?=
 =?us-ascii?Q?0pHHf6fmGpFBBZQIN0WTABYHKlWKrjffqJbm96CtRESLp6BoMofPB17G7fWv?=
 =?us-ascii?Q?bDSGSFp1Yhq3vddtEUldvhfhDbn67pB91mvDlTfNXJJWp1PJogLZozy3Aovb?=
 =?us-ascii?Q?xZ85QjbjJpkhruaotK1uv4q1VI7h33NJwHwjEdhpJi+gh/XMo4u5USrvuqsk?=
 =?us-ascii?Q?Ps/M7LCxzLdwL5ABBU/mchELpbNkJHe1+ONtHE6g+5ZxI5WaZnYa+6mLDAy3?=
 =?us-ascii?Q?uRy07YJtFBgaFSv3X6Gl1tsZB4183u0ci0U888owNl3swFIL4YAlKXHjN9Gj?=
 =?us-ascii?Q?eWy6Fov0hKUWNKd+30JWexGNVLPeOrukOOgMSUO1k0nAsT1qw3LUs9T5APlU?=
 =?us-ascii?Q?Q4C5oNsax4MLV2754X1sxWAQ3cmCJ3C9Tn2MCtW7oYQejvlRoty/gcplQ+in?=
 =?us-ascii?Q?FBWAhC0V1rmPV5uiRdGYMwBjidM4xfk0OvwFkM7WXCPzAd6vjfMbJ4/By78V?=
 =?us-ascii?Q?78cfHCFylRc72mxNa+OpZYKojVdEs3ZGe9362dhCa52QLQ3Co6tuSwbVWuFq?=
 =?us-ascii?Q?q0HDBszSPk46fWioQwxiX0GorcU1mC2ERH3YIpUFCqLqfncwoNkSP9jqIPvw?=
 =?us-ascii?Q?FEVyEUz3+1fE7UXxVJREyOj8u4VTHdXfbN7FHyjmNNov03hxbiqL/qJSUN+F?=
 =?us-ascii?Q?Y8PYL1oc7yfEwoHL4wAB0XMJWxwjdAjuyQqlJNx4VzqxX0MutdyNJrWyXwTc?=
 =?us-ascii?Q?qTv/nXPQc248hxRbP03DL3u8Y6jz8W3XqZaiOA27+tuLlVjjkL3iOBQeTA5H?=
 =?us-ascii?Q?5jKeaE8r8jOxuGIYaIFs9GNAfXz8uO/LCAghd9k7DfpunZaXwaL/QjbaugVt?=
 =?us-ascii?Q?8Bcn7vtn8rm17BKUPJY7jRblNv5+qu60iDWeFuYCZxkAptynAWIA0RWgrWKj?=
 =?us-ascii?Q?sCBsx7Rh9Xy0OCV8HtH798DPYN2eM0GNGW2y20Iz276cqj1S1JEwT+S4m0A9?=
 =?us-ascii?Q?ebRpZS13loife6RkL3vpUOp0NBVKg1bZpxtloZvArZwMsv559E6LQmP1eKQy?=
 =?us-ascii?Q?aM2jUZOfJZVmWnv4M4VSzj40aqII+0H6XwTVdKS0023NzX91gOm5wjYWUxvb?=
 =?us-ascii?Q?OgKYuiZKURSIkuYi40IxOyUjv375SkWk71IXW6obb9J0obAphaSywfJKTh+A?=
 =?us-ascii?Q?to1i2UioLs3vFOT2ZxjhJKOgfY2KkERglESz9GDL3GQ1sDFOkc+cMTZD0Cm0?=
 =?us-ascii?Q?4Oi2yyb6bBtGHXP25xxbaNSVf+xPLGlw12CauLTq6GJLIua6RMR2sjBLCSQJ?=
 =?us-ascii?Q?gTqgc+6UaIyTkZW47Njp4dCpCbGJzU1gna8TLV/mfQV0Ca2WKcF4liUExFAi?=
 =?us-ascii?Q?vi97v3S5mNLlDmouXO1y5bh5CSbS4bSQm8a77BiV5qv/xoo15jK6jy5LTY0/?=
 =?us-ascii?Q?4AtSWo0BuTYHw02Rp9xuCVTbe88k8+cLP2s/r8iyq/QaexYOP/qPsOFGNaaP?=
 =?us-ascii?Q?FCldv5uxUnx4Jr+nNqG/xWuQjYS++VRUdlP7FJeWPe/J7/R6fx1hntZ8zVwt?=
 =?us-ascii?Q?sU2pPUZZwQF5xyEhYZhPOwfuMEPp1kI=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6D36C32849E6324B9300C59CDF386CF9@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3ddf6475-efdd-4675-83f1-08da54820eb0
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jun 2022 19:04:39.4652
 (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: /qJ9c9Y0WKGbjoyUAK79nj9dsc0yQ46CAwWLCgq9YFxI9iAQi7QSmOWfIFTnlBKNn1rXAoMFeYi6ZSpCvYsUMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR10MB1628
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-06-22_06:2022-06-22,2022-06-22 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0
 mlxlogscore=662 suspectscore=0 adultscore=0 phishscore=0 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2204290000 definitions=main-2206220089
X-Proofpoint-ORIG-GUID: BMuYVZw1kYQ9OH4_8kO4lU8WBY4PIKbj
X-Proofpoint-GUID: BMuYVZw1kYQ9OH4_8kO4lU8WBY4PIKbj
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jun 22, 2022, at 2:36 PM, Wang Yugui <wangyugui@e16-tech.com> wrote:
>=20
> Hi,
>=20
> fstests generic/531 triggered a panic on kernel 5.19.0-rc3 with this
> patchset.

As I mention in the cover letter, I haven't tried running generic/531
yet -- no claim at all that this is finished work and that #386 has
been fixed at this point. I'm merely interested in comments on the
general approach.


> [  405.478056] BUG: kernel NULL pointer dereference, address: 00000000000=
00049

The "RIP: " tells the location of the crash. Notice that the call
trace here does not include that information. From your attachment:

[  405.518022] RIP: 0010:nfsd_do_file_acquire+0x4e1/0xb80 [nfsd]

To match that to a line of source code:

[cel@manet ~]$ cd src/linux/linux/
[cel@manet linux]$ scripts/faddr2line ../obj/manet/fs/nfsd/filecache.o nfsd=
_do_file_acquire+0x4e1
nfsd_do_file_acquire+0x4e1/0xfc0:
rht_bucket_insert at /home/cel/src/linux/linux/include/linux/rhashtable.h:3=
03
(inlined by) __rhashtable_insert_fast at /home/cel/src/linux/linux/include/=
linux/rhashtable.h:718
(inlined by) rhashtable_lookup_get_insert_key at /home/cel/src/linux/linux/=
include/linux/rhashtable.h:982
(inlined by) nfsd_file_insert at /home/cel/src/linux/linux/fs/nfsd/filecach=
e.c:1031
(inlined by) nfsd_do_file_acquire at /home/cel/src/linux/linux/fs/nfsd/file=
cache.c:1089
[cel@manet linux]$

This is an example, I'm sure my compiled objects don't match yours.

And, now that I've added observability, you should be able to do:

  # watch cat /proc/fs/nfsd/filecache

to see how many items are in the hash and LRU list while the test
is running.


> [  405.608016] Call Trace:
> [  405.608016]  <TASK>
> [  405.613020]  nfs4_get_vfs_file+0x325/0x410 [nfsd]
> [  405.618018]  nfsd4_process_open2+0x4ba/0x16d0 [nfsd]
> [  405.623016]  ? inode_get_bytes+0x38/0x40
> [  405.623016]  ? nfsd_permission+0x97/0xf0 [nfsd]
> [  405.628022]  ? fh_verify+0x1cc/0x6f0 [nfsd]
> [  405.633025]  nfsd4_open+0x640/0xb30 [nfsd]
> [  405.638025]  nfsd4_proc_compound+0x3bd/0x710 [nfsd]
> [  405.643017]  nfsd_dispatch+0x143/0x270 [nfsd]
> [  405.648019]  svc_process_common+0x3bf/0x5b0 [sunrpc]
>=20
> more detail in attachment file(531.dmesg)
>=20
> local.config of fstests:
> 	export NFS_MOUNT_OPTIONS=3D"-o rw,relatime,vers=3D4.2,nconnect=3D8"
> changes of generic/531
> 	max_allowable_files=3D$(( 1 * 1024 * 1024 / $nr_cpus / 2 ))

Changed from:

	max_allowable_files=3D$(( $(cat /proc/sys/fs/file-max) / $nr_cpus / 2 ))

For my own information, what's $nr_cpus in your test?

Aside from the max_allowable_files setting, can you tell how the
test determines when it should stop creating files? Is it looking
for a particular error code from open(2), for instance?

On my client:

[cel@morisot generic]$ cat /proc/sys/fs/file-max
9223372036854775807
[cel@morisot generic]$

I wonder if it's realistic to expect an NFSv4 server to support
that many open files. Is 9 quintillion files really something
I'm going to have to engineer for, or is this just a crazy
test?


> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2022/06/23
>=20
>> This series overhauls the NFSD filecache, a cache of server-side
>> "struct file" objects recently used by NFS clients. The purposes of
>> this overhaul are an immediate improvement in cache scalability in
>> the number of open files, and preparation for further improvements.
>>=20
>> There are three categories of patches in this series:
>>=20
>> 1. Add observability of cache operation so we can see what we're
>> doing as changes are made to the code.
>>=20
>> 2. Improve the scalability of filecache garbage collection,
>> addressing several bugs along the way.
>>=20
>> 3. Improve the scalability of the filecache hash table by converting
>> it to use rhashtable.
>>=20
>> The series as it stands survives typical test workloads. Running
>> stress-tests like generic/531 is the next step.
>>=20
>> These patches are also available in the linux-nfs-bugzilla-386
>> branch of
>>=20
>>  https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git=20
>>=20
>> ---
>>=20
>> Chuck Lever (30):
>>      NFSD: Report filecache LRU size
>>      NFSD: Report count of calls to nfsd_file_acquire()
>>      NFSD: Report count of freed filecache items
>>      NFSD: Report average age of filecache items
>>      NFSD: Add nfsd_file_lru_dispose_list() helper
>>      NFSD: Refactor nfsd_file_gc()
>>      NFSD: Refactor nfsd_file_lru_scan()
>>      NFSD: Report the number of items evicted by the LRU walk
>>      NFSD: Record number of flush calls
>>      NFSD: Report filecache item construction failures
>>      NFSD: Zero counters when the filecache is re-initialized
>>      NFSD: Hook up the filecache stat file
>>      NFSD: WARN when freeing an item still linked via nf_lru
>>      NFSD: Trace filecache LRU activity
>>      NFSD: Leave open files out of the filecache LRU
>>      NFSD: Fix the filecache LRU shrinker
>>      NFSD: Never call nfsd_file_gc() in foreground paths
>>      NFSD: No longer record nf_hashval in the trace log
>>      NFSD: Remove lockdep assertion from unhash_and_release_locked()
>>      NFSD: nfsd_file_unhash can compute hashval from nf->nf_inode
>>      NFSD: Refactor __nfsd_file_close_inode()
>>      NFSD: nfsd_file_hash_remove can compute hashval
>>      NFSD: Remove nfsd_file::nf_hashval
>>      NFSD: Remove stale comment from nfsd_file_acquire()
>>      NFSD: Clean up "open file" case in nfsd_file_acquire()
>>      NFSD: Document nfsd_file_cache_purge() API contract
>>      NFSD: Replace the "init once" mechanism
>>      NFSD: Set up an rhashtable for the filecache
>>      NFSD: Convert the filecache to use rhashtable
>>      NFSD: Clean up unusued code after rhashtable conversion
>>=20
>>=20
>> fs/nfsd/filecache.c | 677 +++++++++++++++++++++++++++-----------------
>> fs/nfsd/filecache.h |   6 +-
>> fs/nfsd/nfsctl.c    |  10 +
>> fs/nfsd/trace.h     | 117 ++++++--
>> 4 files changed, 522 insertions(+), 288 deletions(-)
>>=20
>> --
>> Chuck Lever
>=20
> <531.dmesg>

--
Chuck Lever