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:     Dai Ngo <dai.ngo@oracle.com>
CC:     "jlayton@kernel.org" <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/1] NFSD: replace delayed_work with work_struct for
 nfsd_client_shrinker
Thread-Topic: [PATCH 1/1] NFSD: replace delayed_work with work_struct for
 nfsd_client_shrinker
Thread-Index: AQHZJhnRQiYIyIlXYE21938HiXynGa6aEgIA
Date:   Thu, 12 Jan 2023 02:39:34 +0000
Message-ID: <9160FACD-7A16-48BF-B4E8-F782566DD837@oracle.com>
References: <1673482011-27110-1-git-send-email-dai.ngo@oracle.com>
In-Reply-To: <1673482011-27110-1-git-send-email-dai.ngo@oracle.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?+iJU1Wyof9nOD13QlBuig0/ev8aTRApY2CG665bV6Qg9AFZwPcwdeGahzA87?=
 =?us-ascii?Q?FJSeCafJ97WgWX+Qmy180XDtG35uKxOjM7m0HliTg0HJF+wTantmYCgJuxlQ?=
 =?us-ascii?Q?CNJ0GkbttdvUAgdDXUYvhlvXG/Uz3VJHec9mbO80Ni1g1Kxe975mUwFhYZt2?=
 =?us-ascii?Q?R3KDQfQZvlVvmbZv7q2OUuigQUBC46Gg7df34u6jad8aSIDedXmRJl25wYxM?=
 =?us-ascii?Q?OXsNthqQzKHq/i9eUM6FHx175v4dnujknuo6q5WCXf8t8Ck+n19Q7cczYGdD?=
 =?us-ascii?Q?bOPUp2aH5B0FFnABfdyR7HfbFSV5RV9OSD+D+A0ygVbIukTmXGcCmn+3FPWT?=
 =?us-ascii?Q?PKckPyvBgFPMwSF7PYcH7EhWpOPXcqw7aQ+ROEC9xe0vBu0LUZ2rReXELRrA?=
 =?us-ascii?Q?7VwO1i3A7c8G9mOKyd4dDWfmCuS+Ywqu6ijQU3DEdmgxjLv1wxmoY3UWuIui?=
 =?us-ascii?Q?BUqUBTSYyeWQgBZE/+b1FquOjZH/3rGqeILcYI+cfwrARaxFDRTctu+H9MGl?=
 =?us-ascii?Q?t77iey6OsXgHifpRhwhN6uRqQavGlN6q9rf/JtiRT0MZU+qcGeeAtkhYLepO?=
 =?us-ascii?Q?FXKgkFwy3pn1sS/6X38WWBhelZNDf9CwcjagqrXsVHsGCfCFNAwUVQKpl7wk?=
 =?us-ascii?Q?Ah0ETutj3XXZoqIC/TZoWGPvTyw5vwF93bWMm24t5yUZSyWepMIGzXfOanza?=
 =?us-ascii?Q?/aoy5i+6EtaZj8+awsg57QaHcFp286/pV9wBhngqNASfRP6anOLIZZdN4MrI?=
 =?us-ascii?Q?LDJEBKbwVzBnAr/n6DC4MzMisoTD2eaTjTsv4DLGBdZ/nUyYrdXVy9hEaws/?=
 =?us-ascii?Q?hQHloyO6CqMj93C8kl2WBRvhRgbW7WSofvWtlKHuLQ7qGIHXiMTGw3ILoTv2?=
 =?us-ascii?Q?IYHCkKnm5YwupcdbY4gQv7Is7Qxqx3ggdlQ0Ph3YZDTQ+uw2e8S2RYY6umqh?=
 =?us-ascii?Q?8BSNmmrKZIfRFVsvz1pioDkkJjbwKA8LPTE6lOujUJ6QTw71qAAp15GneZ/t?=
 =?us-ascii?Q?ovpmBsdCvtLKClRoFt0tJCUmGWuSCJs2f9IVHGNkXcz8j/A78+oQlQwGares?=
 =?us-ascii?Q?NtuxuYP3rDiCrQcQir8j7cDsYd4ycxuMEqpUnFWDU4lEMxNlAAkGDXJUzO8/?=
 =?us-ascii?Q?ltUlHK5nuRe2Xx4eoT8GJPRzt5V4IjmynFWZFmT/Wqa/C/USfxWVpOoTLk0s?=
 =?us-ascii?Q?OkXVy4m9K+JhSQZe1+acu93a6Av6rT0HjE53BJ0WzOOngsBFIvPBaGPThXL/?=
 =?us-ascii?Q?7DaTbXxdObh0SQ/m++4mE/td5nFMW2NAThgekgNy3/lvB/KdJ60rO5UHlHpY?=
 =?us-ascii?Q?MePvnnsrLFBd2I4EBbRUCUsBgS4u8scUC7GL/stMfX7xVVGv73RGknY4ZC42?=
 =?us-ascii?Q?MJOXL0xyVaewIbpbCBpANjMbkg4XryzLAOQagojPkc4eZxiNl4F+dpNfuzRA?=
 =?us-ascii?Q?xfLYoOhDcl5zZVIz77G1mcnBkURQiWkrLV2aNF0iAGVG0wwIglTywFRXCPz9?=
 =?us-ascii?Q?AbV0TQVDjZn12HPyqb6iT5MsvF4W7UyY4AVh9WULH1p/wv4+Knz4wZ+Aye49?=
 =?us-ascii?Q?b7Y6WV7AKgBj7f4+51JrJ38zi8rao1vL1uBvVoBlCxlqHc0Cib6kyz+cewx0?=
 =?us-ascii?Q?kA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1192C70BC045894AB7BCF1753A3B77AB@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: nir0lCajeF4MmxTVjVBGexGQuNgap+G8s8b2jZoIhhG8yzkyY6CGHfcFb8taj4nBz7unUDlJ4MeRzVatEhwKkV3JUp9AIgUY/7zBE3lk0GnQBavy9KAoAVM0R4VjIGXyzqSXH2HT/uKjDbhvaROd80JgqR2R6/iDu0qZ+VwMMSWjmx5t4t/JHLbu+zOqVw8hotk3EUKD+nFmj5erKRRtqGf/h+J0aMRzO4OybC4xX1Iz38oMmHdTHdZNUH/9jkCVjtW1HiGok578mg6kaCWXu6KnatqxbfWVXW8hmNvWadvr13lnJOkzVzk159uJqp8/zCeuBwzEt8CxxjfEF/u8okzhrAQdnPZ5PaPFyh+fD4vIFd1JLIQyLdvR7PqLftiywzILKbcUYskUPqJ9lSVCbLiobyzXg6ZZ8/UI2QrnyBssv8Vlm00t0xT0lpiLpELvMjFLm5OSEOvFMSDoTVoxS9gBk2Vuv1o3X4FFzsfcSOTs0PG9wkKNfOZa4GL3XbU3e/WQxKgUKXNlMdEhYuuXsf8EfVGeoEaA8y1BiBM27N8Wby4L62BGav70Ym79GUVcqMmT+NTrt8ULsRP6oVh3wlc95zbUM0IEcgRtN+EY/3fdFRuF7BgzsZB1ZqCA/OGEbDlVB7fLE2ODhBiywOcOhcdWCt68slV0GSdpOi3w3vVzUDiQogjW6nwzKiQ5uzLFBptgqP5+WypU3NuvlO9uZWtcudcCC1rqg+IGiE4xePhZ0Zm+sSSiM8ltUoD5HxpS+p3/n/MOf4Vi8rHSIEMgKTkHzacZwWoWea0+nufs1tA=
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: 6b20ba61-8dce-4d47-0f93-08daf4463db9
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jan 2023 02:39:34.6304
 (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: 1IgOLI2Rx08KaSeQ8ZHIvappE8VEnfJ9ofui5JXYSYWI2DWXrrCh+Tcs6AiVNyZqD9ucWuwCdvveLPRYrsXpEA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4233
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2023-01-11_10,2023-01-11_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0
 adultscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 phishscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301120016
X-Proofpoint-GUID: 1RSVR2CDTpDVdnNU2q0CUoXL0wNqeu_J
X-Proofpoint-ORIG-GUID: 1RSVR2CDTpDVdnNU2q0CUoXL0wNqeu_J
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 Jan 11, 2023, at 7:06 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>=20
> Since nfsd4_state_shrinker_count always calls mod_delayed_work with
> 0 delay, we can replace delayed_work with work_struct to save some
> space and overhead.
>=20
> Also add the call to cancel_work after unregister the shrinker
> in nfs4_state_shutdown_net.
>=20
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>

The missing cancel_work seems important enough to apply to for-rc
instead of waiting for the next merge window. Thanks for the patch!


> ---
> fs/nfsd/netns.h     | 2 +-
> fs/nfsd/nfs4state.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>=20
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 8c854ba3285b..51a4b7885cae 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -195,7 +195,7 @@ struct nfsd_net {
>=20
> 	atomic_t		nfsd_courtesy_clients;
> 	struct shrinker		nfsd_client_shrinker;
> -	struct delayed_work	nfsd_shrinker_work;
> +	struct work_struct	nfsd_shrinker_work;
> };
>=20
> /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a7cfefd7c205..21bee33bc6dc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4411,7 +4411,7 @@ nfsd4_state_shrinker_count(struct shrinker *shrink,=
 struct shrink_control *sc)
> 	if (!count)
> 		count =3D atomic_long_read(&num_delegations);
> 	if (count)
> -		mod_delayed_work(laundry_wq, &nn->nfsd_shrinker_work, 0);
> +		queue_work(laundry_wq, &nn->nfsd_shrinker_work);
> 	return (unsigned long)count;
> }
>=20
> @@ -6233,8 +6233,7 @@ deleg_reaper(struct nfsd_net *nn)
> static void
> nfsd4_state_shrinker_worker(struct work_struct *work)
> {
> -	struct delayed_work *dwork =3D to_delayed_work(work);
> -	struct nfsd_net *nn =3D container_of(dwork, struct nfsd_net,
> +	struct nfsd_net *nn =3D container_of(work, struct nfsd_net,
> 				nfsd_shrinker_work);
>=20
> 	courtesy_client_reaper(nn);
> @@ -8064,7 +8063,7 @@ static int nfs4_state_create_net(struct net *net)
> 	INIT_LIST_HEAD(&nn->blocked_locks_lru);
>=20
> 	INIT_DELAYED_WORK(&nn->laundromat_work, laundromat_main);
> -	INIT_DELAYED_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker)=
;
> +	INIT_WORK(&nn->nfsd_shrinker_work, nfsd4_state_shrinker_worker);
> 	get_net(net);
>=20
> 	nn->nfsd_client_shrinker.scan_objects =3D nfsd4_state_shrinker_scan;
> @@ -8171,6 +8170,7 @@ nfs4_state_shutdown_net(struct net *net)
> 	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
>=20
> 	unregister_shrinker(&nn->nfsd_client_shrinker);
> +	cancel_work(&nn->nfsd_shrinker_work);
> 	cancel_delayed_work_sync(&nn->laundromat_work);
> 	locks_end_grace(&nn->nfsd4_manager);
>=20
> --=20
> 2.9.5
>=20

--
Chuck Lever