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>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2] nfsd: simplify the delayed disposal list code
Thread-Topic: [PATCH v2] nfsd: simplify the delayed disposal list code
Thread-Index: AQHZbxiG/N+K790Z6EWLNCNAtkF8za8s/ZYA
Date:   Sat, 15 Apr 2023 22:59:20 +0000
Message-ID: <8D55FAFA-FD29-4264-9232-F92D8A72C15C@oracle.com>
References: <20230414213144.385947-1-jlayton@kernel.org>
In-Reply-To: <20230414213144.385947-1-jlayton@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?lqwRS+nO/M6lnF5wmvaBQ0kOtWV7wN7lQURW9kpBaSlmn00IUGCzjGTiGcw3?=
 =?us-ascii?Q?6EsHCv/jyThEe3xqKQpRFHy5EuHg5Z+IYB71kwYkKXufu/skNA3gQONRFyNJ?=
 =?us-ascii?Q?PqPDPebEZN86cCwJSa/W6N3vderBUju/+g4IhYRrzRqj+aWtcBc3zpuhTkkF?=
 =?us-ascii?Q?bQkpP5SFBwbdbvVfU56GtjX5EDPZnG8TEVqjWyLyLeTtz0mFLj7BC+UDvdMc?=
 =?us-ascii?Q?qYPrCs9qDEw6LS8i7zMGf4V4OgtsjzsobIh7lYlAMoIXQKwQgG/Dz93h8NN9?=
 =?us-ascii?Q?F4TlcVtWHwCh8QDgBwsI0K/1tiXAVN4jiV8xaT21pznYEIyGr/9jcBxvEy9t?=
 =?us-ascii?Q?e/Ntl0I1li5734ySDAb+Z47g5Ehg4/fIByKes528y/Je6YYUVC0vSOI/Ii/J?=
 =?us-ascii?Q?gMa9x9oz9WzwC7pMFvuKsHJe9EJupXcvvKKJaQvj2fiYUnvhPOmcoB4R2Iqa?=
 =?us-ascii?Q?FzQU2FdW+yK0zkDnY7FvzXNq8QhLYPEDQllYINcBl1pEGCEQV9U9XX7vKo2g?=
 =?us-ascii?Q?CXzntHbnajv04hEq7y8LV6mxbEnN6W5bLbsxaozdtgqcHzbxSyVdFefUE3+D?=
 =?us-ascii?Q?C2/AX3QXBhLla2s4MF6Lc22CWTPSKFB8r2a6eLlZDGMiM2U444YxCOWysL8V?=
 =?us-ascii?Q?/sRhsrRGB51FBs1csvJP9Lb+kdeAnqYBD55rVQB0WJa1pGvZ3Boo7eGD1d8K?=
 =?us-ascii?Q?/p6/ljDuRsKxstZduBTR+X+hUBYRAY8TC6LqhlUabOQaWaT76fMJ9DSFpVSA?=
 =?us-ascii?Q?nEFfGFvjgt0DGVjp9hBJF2iRJ6rGRt44QI+1dveWv6Jhlmc/DE6OeW+vO1iU?=
 =?us-ascii?Q?PMN3qrzrECxS/toEB+hXKLBZjVr6KkGa6uU4SUvopuk0PkhyKD/r8K1+M2uZ?=
 =?us-ascii?Q?WnaJ7pjhWfzQik4BqYv3+m6cSW6IcqYbcHtMT/gLfjXdU62o5NQRAFQwhkCR?=
 =?us-ascii?Q?cVqzn1nCVOAE5QoDxhVkT6CGTSns6/Tjk/8jlG/f2frX3Uha13H5jgHQLLMp?=
 =?us-ascii?Q?IxaU+UxpaB39PewS++go9C/sULwJJHE0jXK1NB7pBovewHfeWD3CE9ctjzzo?=
 =?us-ascii?Q?J42noirnrAO3b9GBTPPuP9WQQRfiXuLtThmf6ek4ze5P+tAqx/Z66y5/UAuq?=
 =?us-ascii?Q?QvJQ7Jer51UaMC12G6JBOgt3m2mTxSjb1NkLQYa4VTscX6tl45Tjckc0hIHv?=
 =?us-ascii?Q?6Xfspnk0ioiOarmMGWFNa67OFp/YTxrKFt1J7D0d1RsN6yC/HQuj2VCqOgDS?=
 =?us-ascii?Q?TmWLn1EBprsRGCofUcu0dZAnOftpM8HeyiruoV8PSH4iFRYIh/lMjwJBtkXk?=
 =?us-ascii?Q?nQRbYruvlLFPlphXX34i0D1iYoM+nTOTYRz/+WjdJYrL2q5jVk4MOZIknCQD?=
 =?us-ascii?Q?C3/LuhOIMrOU7AhkGYh95IaRhtbmCOKh5vyqi7dxdzQ/cW+6wY4dke6dQXSg?=
 =?us-ascii?Q?VDR8wP+rt82fX0McqUtmBgs0HoBWc7Wncwzx9X4QL2c19NQvW3eO1oVn6ob3?=
 =?us-ascii?Q?Kwdr9yBnQoLBj/eWRr2OdeGsVmoS67rghrdl46LNIjVLGEarEWb6xDnxCoZU?=
 =?us-ascii?Q?3RQTHwqPNl8RqV31rWzGRklXnkBCTMwwamdcccQtmTzJ90sQrO+gKRWIcOED?=
 =?us-ascii?Q?/Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <852331DE9C45094FB5F2E8DAF598FA24@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: DWA4GFzkINLhJgZu2nD1wAEnjnj3M5u5bRIqx6icH+orD8CxOhRooasMHM8iGaBuqSIWa5Le84H88f0KzZK/gyOYOajZxPPvms1zpJtuAikEr/LB6olQPV14xyX03jUZdZhev1guMjsfynwwNrZfCh01/bKcIk3vxlxwlA+w88ohXeolwnFZkDkXHLwhU0kDQQP0sAWfRv9rdMUNxEap7bW4nWe3wQAiD60qvR0G4xb+Uz13bm5ue981c35rufyf6xGgjbeEzKEInheBRqXuNDNr1j7cTcSPcCjs19jELyDu9m7CEN1XHi1kx47RuFAkhbFEpUo4G39RsP46PSDvd5kCBWd0KncQA+GzvN7Yjx7xporrRZR5Ke/KoFeuQAKwIwIJq0d7fCRh13s40TSi7lenMcKMpeWLhIpdhc/n0z+eG9jBZWT4O/AmSK8cATxzev7a5KPQgPINdPpkVt25WmXovSzIcH/Prza0cwrr7+sA0DD8PVnxzwoAQ10WLrNUuwrrxvFAUUrbTh/Ko0MIfufGyBd5CWCYAh0mh0IxA71rW1LQkKq1lnoqoH/G21l0yz5PpuFbtjZVTxjCJMlfUtT5hsgd9LB5uKziBz+JdyONf0BtUY8GvZzUdEfgGsnKmIKcQ2hlm16buabEFh9h5i2m1gHiX4+9zrrborWotxzNoWjjRup1gFK+fOmYt3Migun9fdOzRfpoqTizynoc3SG75V+cWGUyeBheZyx6E+GUbsQ9m+ipJpF0DogM/VrPAOIN9sLKzYKvSkKH1gYGNO/8bpjZeXvbo6MZ0hjW02k=
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: 077a829d-a121-4c2c-a343-08db3e050c8b
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Apr 2023 22:59:20.8803
 (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: oDtXhbCU1WwKNG2HvJ8NMxdROpDNXLe5BewUFLE5NNwLst28jovrP6voZjJKRw2I4XnQn3iTP3DIFBfDbCQKRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5216
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-04-15_11,2023-04-14_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 adultscore=0
 malwarescore=0 mlxlogscore=999 mlxscore=0 bulkscore=0 phishscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2304150214
X-Proofpoint-GUID: 0aPoDxKD-A9LntEql_rAmB9Oq8dATxNf
X-Proofpoint-ORIG-GUID: 0aPoDxKD-A9LntEql_rAmB9Oq8dATxNf
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 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 Apr 14, 2023, at 5:31 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> When queueing a dispose list to the appropriate "freeme" lists, it
> pointlessly queues the objects one at a time to an intermediate list.
>=20
> Remove a few helpers and just open code a list_move to make it more
> clear and efficient. Better document the resulting functions with
> kerneldoc comments.
>=20
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> fs/nfsd/filecache.c | 64 ++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 42 deletions(-)
>=20
> v2: add back missing queue_work call

Thank you! Applied to nfsd-next.


> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 2f0b2d964cbb..f40d8f3b35a4 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -402,49 +402,26 @@ nfsd_file_dispose_list(struct list_head *dispose)
> }
> }
>=20
> -static void
> -nfsd_file_list_remove_disposal(struct list_head *dst,
> - struct nfsd_fcache_disposal *l)
> -{
> - spin_lock(&l->lock);
> - list_splice_init(&l->freeme, dst);
> - spin_unlock(&l->lock);
> -}
> -
> -static void
> -nfsd_file_list_add_disposal(struct list_head *files, struct net *net)
> -{
> - struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> - struct nfsd_fcache_disposal *l =3D nn->fcache_disposal;
> -
> - spin_lock(&l->lock);
> - list_splice_tail_init(files, &l->freeme);
> - spin_unlock(&l->lock);
> - queue_work(nfsd_filecache_wq, &l->work);
> -}
> -
> -static void
> -nfsd_file_list_add_pernet(struct list_head *dst, struct list_head *src,
> - struct net *net)
> -{
> - struct nfsd_file *nf, *tmp;
> -
> - list_for_each_entry_safe(nf, tmp, src, nf_lru) {
> - if (nf->nf_net =3D=3D net)
> - list_move_tail(&nf->nf_lru, dst);
> - }
> -}
> -
> +/**
> + * nfsd_file_dispose_list_delayed - move list of dead files to net's fre=
eme list
> + * @dispose: list of nfsd_files to be disposed
> + *
> + * Transfers each file to the "freeme" list for its nfsd_net, to eventua=
lly
> + * be disposed of by the per-net garbage collector.
> + */
> static void
> nfsd_file_dispose_list_delayed(struct list_head *dispose)
> {
> - LIST_HEAD(list);
> - struct nfsd_file *nf;
> -
> while(!list_empty(dispose)) {
> - nf =3D list_first_entry(dispose, struct nfsd_file, nf_lru);
> - nfsd_file_list_add_pernet(&list, dispose, nf->nf_net);
> - nfsd_file_list_add_disposal(&list, nf->nf_net);
> + struct nfsd_file *nf =3D list_first_entry(dispose,
> + struct nfsd_file, nf_lru);
> + struct nfsd_net *nn =3D net_generic(nf->nf_net, nfsd_net_id);
> + struct nfsd_fcache_disposal *l =3D nn->fcache_disposal;
> +
> + spin_lock(&l->lock);
> + list_move_tail(&nf->nf_lru, &l->freeme);
> + spin_unlock(&l->lock);
> + queue_work(nfsd_filecache_wq, &l->work);
> }
> }
>=20
> @@ -665,8 +642,8 @@ nfsd_file_close_inode_sync(struct inode *inode)
>  * nfsd_file_delayed_close - close unused nfsd_files
>  * @work: dummy
>  *
> - * Walk the LRU list and destroy any entries that have not been used sin=
ce
> - * the last scan.
> + * Scrape the freeme list for this nfsd_net, and then dispose of them
> + * all.
>  */
> static void
> nfsd_file_delayed_close(struct work_struct *work)
> @@ -675,7 +652,10 @@ nfsd_file_delayed_close(struct work_struct *work)
> struct nfsd_fcache_disposal *l =3D container_of(work,
> struct nfsd_fcache_disposal, work);
>=20
> - nfsd_file_list_remove_disposal(&head, l);
> + spin_lock(&l->lock);
> + list_splice_init(&l->freeme, &head);
> + spin_unlock(&l->lock);
> +
> nfsd_file_dispose_list(&head);
> }
>=20
> --=20
> 2.39.2
>=20

--
Chuck Lever