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:     Jeff Layton <jlayton@kernel.org>,
        Dave Chinner <david@fromorbit.com>,
        Trond Myklebust <trondmy@hammerspace.com>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>, "tgraf@suug.ch" <tgraf@suug.ch>
Subject: Re: [PATCH v3 00/32] Overhaul NFSD filecache
Thread-Topic: [PATCH v3 00/32] Overhaul NFSD filecache
Thread-Index: AQHYkvgwrDJIabsGnUOsgLaSrfA226107HiAgAABKQA=
Date:   Fri, 8 Jul 2022 20:32:04 +0000
Message-ID: <7AD28D60-0238-4981-897E-14BFE0E520BA@oracle.com>
References: <165730437087.28142.6731645688073512500.stgit@klimt.1015granger.net>
 <6927644b409f92ed0622bb0e8a677b8d8007756d.camel@kernel.org>
In-Reply-To: <6927644b409f92ed0622bb0e8a677b8d8007756d.camel@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?qNdvce4SjVfx3FsLC3k3n2l1he/Ig135W7A5mOAfGplug09qgpmplrn34NtQ?=
 =?us-ascii?Q?r0NEJee6oBN8WkGAnVBgl79muG8MFl+6+tqvNjzHdyfCYnC1OHNFQYcqGw/3?=
 =?us-ascii?Q?pPVdM9lSdunAFWn2ftY0qHzeuL5Gnhpilzt6xNZ86pMBUKrB4K5pKq46GHy3?=
 =?us-ascii?Q?4UkNGAZNEY6VSggB+RskSOpRA6w1FRqm8+QWeNdW3hZcUXBo5IfL5aYU4eYS?=
 =?us-ascii?Q?Qg6Vw7FT/wAfunKgT5lHXXQQVFU6VpmW2hlH7TQ78Cx+QsnnC4lxhpG6Ln0m?=
 =?us-ascii?Q?BZKzt4dXvrNI1j+1xTcRiLIjUJ5w5NDPW3pwRsXDzcO+Zu52N1PZFP68bJRd?=
 =?us-ascii?Q?uVRvkXJHaJT46DQqD0eUb+nH9CLk38G2ckIGJuobhIH3ph7dKEDuKLQltJbc?=
 =?us-ascii?Q?NOcvA6x9mPwGEGOSD7Vg0yo2VbDGaoyyuXFxE1iCuvh1XQqPLQP7kjDwxOdO?=
 =?us-ascii?Q?VliAQ/dazFMFGpDeq7mkzVQVD3aStKJEQB3yrA9HdOLfbF6gNv0o4DBkt8D4?=
 =?us-ascii?Q?Xuee4q5qlotYjJvIJpOgggm40LpfS8cdoI3agyOciqoKzFPifcdHnFtTMx1q?=
 =?us-ascii?Q?F4O7CeL/g//fB+BbmAfZcbXwOOxqZ8oOPI6gvektIQXU/ZjyrOyY10RWnFL5?=
 =?us-ascii?Q?KHapbsIPpSiB/H+QyDvQujNZLMuTbJQusxihQb/7j0pGVmdmHjDS9fRiZYyT?=
 =?us-ascii?Q?kAwVdDm9ZnzGWdrXLa5XtbdoPgxRlV5E1RvKoqRmTyb7QzQXNilM+/z5xRAD?=
 =?us-ascii?Q?jK83ov4kOuXQ/iq+gXXWHJTJaiwfXrBMZrh0WQ5eGnl7V86aj+/JrmKFLVWY?=
 =?us-ascii?Q?vMjoXwvsrc/X/CsOwCxAOWPby1QcKGAf4F/5YxSruwtnMkXjuUWhBRQptjyP?=
 =?us-ascii?Q?L9GswMqCI0bf7q9IJ2O3M6hZxL+ChnPOlrdTf9CtP+g6fRajTPq/ibcBJRB8?=
 =?us-ascii?Q?qC45m3xtckdgZwIogwrmivjsXXbvjd5BhxRmYmNZ1k8tmy6DSM0ByB5O+YRA?=
 =?us-ascii?Q?784XkWHBWrt593xskqtxBVGJlPbdZnrPZDcz3fHMgVeG8/eInAuLQtWwo51O?=
 =?us-ascii?Q?nrS+Sf679uyI2ED5BYjSr6j1nEyPAg8jEMhTSWWf+E4N140H5yXZG3bOdGT5?=
 =?us-ascii?Q?rhENJfYlJ1OojlfC7CNH8001WJ4i3rQOMrpptDycugNulZLGjsV1rx0Q2VJK?=
 =?us-ascii?Q?791YsOAgpCZCH0xp/4u++Pps/HgJr9ahHksj7G2ZXcPtbGf+xeeUViJnkKbY?=
 =?us-ascii?Q?A/sO76dPt7c9mF+T7SnhM4T1kB88oRLdg7RTF/jqtHVcO53t2wjKSjartquB?=
 =?us-ascii?Q?7NFyzKkvjNDzeJ+7NtvGHQthYlJ4okPlhzxLt2zTflbBG+wGmDh+IoxwkLKU?=
 =?us-ascii?Q?xtF8E2bP20DO/WxxS2s/ur5Cev0oOrmKv3I+YpLfzFu0vAeCtE1aQHjJBKMN?=
 =?us-ascii?Q?VQwNG25/B2KgJ2J5mrUKLgVfsNJU4aos97cCISK9fOLnqbMvRoS5DcMJgC4X?=
 =?us-ascii?Q?3e0lo2zklEo/kadbsnAqBUxro0TphlVrohzgRiDgUBmTiayoZ9FXK3tdlH+Z?=
 =?us-ascii?Q?FysVaYGXoDXgwt/1Z5qWsDSoeWS0qQKebcRKZr0CZlTo914ODbaS+hlJa0NZ?=
 =?us-ascii?Q?Lg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <75F740B28349A1439EF168E083895633@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: be5487df-8b2b-4df4-6a3b-08da6120ebcf
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Jul 2022 20:32:04.8808
 (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: fc7JAZzDCRxkWwGa4oePPjyaKhzKSMiF5q6SBGrGJH9OpBstopndipvDVr/Xfpl+zjMC//OOeODZ+sPBrR6IHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR10MB3437
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-08_17:2022-07-08,2022-07-08 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 spamscore=0 suspectscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207080080
X-Proofpoint-ORIG-GUID: qqOkQdugBvUIOmb2WoywVQy6mGOWMRbP
X-Proofpoint-GUID: qqOkQdugBvUIOmb2WoywVQy6mGOWMRbP
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 Jul 8, 2022, at 4:27 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> On Fri, 2022-07-08 at 14:23 -0400, Chuck Lever wrote:
>> 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
>> These patches are available in the for-next branch of:
>>=20
>> https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git=20
>>=20
>> Changes since v2:
>> - Fix a cred use-after-free crasher
>> - Fix a Smatch error reported by Dan Carpenter
>> - Replace dereferences of nfsd_file::nf_inode
>> - Further clean-ups and white-space adjustments
>>=20
>> Changes since RFC:
>> - Fixed several crashers
>> - Adjusted some of the new observability
>> - Tests with generic/531 now pass
>> - Fixed bugzilla 387 too, maybe
>> - Plenty of clean-ups
>>=20
>> ---
>>=20
>> Chuck Lever (32):
>>      NFSD: Demote a WARN to a pr_warn()
>>      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: 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: Replace the "init once" mechanism
>>      NFSD: Set up an rhashtable for the filecache
>>      NFSD: Convert the filecache to use rhashtable
>>      NFSD: Clean up unused code after rhashtable conversion
>>      NFSD: Separate tracepoints for acquire and create
>>      NFSD: Move nfsd_file_trace_alloc() tracepoint
>>      NFSD: Update the nfsd_file_fsnotify_handle_event() tracepoint
>>      NFSD: NFSv4 CLOSE should release an nfsd_file immediately
>>      NFSD: Ensure nf_inode is never dereferenced
>>=20
>>=20
>> fs/nfsd/filecache.c       | 727 ++++++++++++++++++++++++--------------
>> fs/nfsd/filecache.h       |   7 +-
>> fs/nfsd/nfs4proc.c        |   6 +-
>> fs/nfsd/nfs4state.c       |   7 +-
>> fs/nfsd/nfsctl.c          |  10 +
>> fs/nfsd/trace.h           | 300 +++++++++++++---
>> include/trace/events/fs.h |  37 ++
>> 7 files changed, 774 insertions(+), 320 deletions(-)
>>=20
>> --
>> Chuck Lever
>>=20
>=20
> Nice work, Chuck!
>=20
> You can add this to all but #15 (where I still have a question about
> whether adding it to the LRU on every put is the right thing to do).

Thanks for your review! #15 emulates what other list_lru consumers
appear to do, but I'd like to hear from Trond and/or Dave who
advocated for that approach.


> Reviewed-by: Jeff Layton <jlayton@kernel.org>

--
Chuck Lever