From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Neil Brown <neilb@suse.de>
CC:     Bruce Fields <bfields@fieldses.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 05/19] SUNRPC: use sv_lock to protect updates to
 sv_nrthreads.
Thread-Topic: [PATCH 05/19] SUNRPC: use sv_lock to protect updates to
 sv_nrthreads.
Thread-Index: AQHX4AnWWLhNYiXw30KvzT4KsHcZZawRUmIA
Date:   Tue, 23 Nov 2021 16:43:24 +0000
Message-ID: <3877F926-545B-41AF-8C0C-80582E83F1FE@oracle.com>
References: <163763078330.7284.10141477742275086758.stgit@noble.brown>
 <163763097544.7284.15751243743215653521.stgit@noble.brown>
In-Reply-To: <163763097544.7284.15751243743215653521.stgit@noble.brown>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?d7HgDSbAM4VlBVC1yt1apPbVn4tbHmarD3l0wJz6aaoDKoHhjcnHP27+WGtB?=
 =?us-ascii?Q?FtUig1kJMKgxNJATr0FPPuDhvGBUNKfUlsvN4ZXwmcJVLw+cljCEct9wUr69?=
 =?us-ascii?Q?s6s34JT0PImvNUR6KG3nqN2FMs/yOStT7L1c0QAjdI98qcWogo/QK1ti81hF?=
 =?us-ascii?Q?yyjFTuXCYkKuGJCl0cIqiJVUVA+jbmX2Fu1Bp44mcj/gEb3sRJraYCzU+5Vv?=
 =?us-ascii?Q?09qqCM+HuHr3y9ody3sdAYdtaWqHgkrgq1dvVdDkfqF0BOl3d6NLoVgTU142?=
 =?us-ascii?Q?M35IBkPevLkUHt1pGRJ74f/ebNxg4cNov/91aV2VgBnT4rpVVpzVYfHCzaUW?=
 =?us-ascii?Q?jdcahY/VYl58bYQkZVyEkiqzEQaBYCwwwE3UoW0zPtndm43IO3dU6MNQKZSu?=
 =?us-ascii?Q?7lStyHqOo3HRpFCFNljjNSOroqAiLrZVVoGsOGZG+JkQYV2QT1KIG5m6ar1y?=
 =?us-ascii?Q?FxMxuMYqWy0mkOAiBZGXxXGqNqoHJikAXOiTDXQWSjyYbE3LOd9App3QnwnH?=
 =?us-ascii?Q?KCr9NMuVBXySAQ7n3YmSVO3QekDOsQq9P/1And5SDahN0o2xgr+zDEZfivPR?=
 =?us-ascii?Q?5qyhIZs0TW7S9fgo6LAVtEgGh6pTdtBiJSkjaWAL4J4Ya5LsPd/DfEEIQHB6?=
 =?us-ascii?Q?sqSN9kn8gadq/UA7LdLHSm0zfIOGisx5QZO1xNvClFpn85pu5Gph8UY2egth?=
 =?us-ascii?Q?FNuxd4fA/zLiRc10+q5YAq/0YeEyZjhp51Cm9dQa1QWFvCPWjOWSfnVlHxIz?=
 =?us-ascii?Q?WetFG0D5RzfqNmJLnU0NFyQf7epAdxnc7Q8cDxCU4cgESuuAw/0vDBUlKw2M?=
 =?us-ascii?Q?Wzau4BGq1jDHfHp2Lkvz1QVcpohHHuPlLOPdBcmFKYkOlyMNs6xhyPOwro4x?=
 =?us-ascii?Q?S0pH807juc14SDhxMATeqcSJ3B+jd4LA5PQaIcrcSWyNh83WygZN7vXCknQC?=
 =?us-ascii?Q?yE08OClKLsAUa2LP2TEKPC2BPO37cyFxmKTRQoGhxCN3e9pBp3RHaPqaxL+5?=
 =?us-ascii?Q?7NJsg5qgTE3R3w6I+equ29OA0pPXNK4giDM/HoGn96QzExsxermbxVNq4NJG?=
 =?us-ascii?Q?wQS40TxQYhe4gOJp7AcUKWF8BdlIjMIsxDls5CUw6rxh1flwYwRKhWMegV8R?=
 =?us-ascii?Q?dWtNUGhKewji6lZ3JZLOFwFL8DYjz5DYbD0ubS9eIs6WIkJ2DeyDzXh0rXYc?=
 =?us-ascii?Q?Xc3ED/zQ28pz6gvqg7R7eug5Ow8tZEOIblCu4dPjbotdSHWF28D81t4xFKhh?=
 =?us-ascii?Q?wEiHBd/oXVw1eJTvH4Fr/mBeuiNv5jTNJTp2cT1MuFFHPUzRFTinrQdSwuHq?=
 =?us-ascii?Q?++bZV6M7RCplLYVuDoMf8qKz6lr91swKZKr8brNYduAr0hw0el7Az2o68VsX?=
 =?us-ascii?Q?BM1EsrEOIwj4koKTDXHfrlFAve3m5N0HdsI2kBzzqYSwT0jnwFvXZEB5SgWM?=
 =?us-ascii?Q?xn88xfx54YtbgXbp8sQFjMJu43zoe2rnBMGBP35pZQOjGpmERo7XakZ+V3MP?=
 =?us-ascii?Q?DbIiioU/HuuXIBo0lb6SOzqyEnfPs5ZJmhUquYsfkczy9GQTXlCzq04KqYfi?=
 =?us-ascii?Q?dF4m3JZVAM5KFvMk7+zByKAEcM/thHlVIyfz3t+kkUQI0PMDxX9mNugafYeF?=
 =?us-ascii?Q?tNsbqZjlunjh2unz9QmmebA=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <123FF15F833A5940999B4580EDFC4BAA@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: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6c3b841d-6ac4-4e96-480e-08d9aea05e36
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Nov 2021 16:43:24.7495
 (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: s0DBWkLIfbhGmwlDupKU/WfA+1jRpTtUdXo0ZJ8++i0/4qUhqXaas8KTwjkIYVOLLcMbeMW55sPXmGBWJrR+NA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3720
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10177 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 spamscore=0
 bulkscore=0 suspectscore=0 mlxscore=0 mlxlogscore=999 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111230084
X-Proofpoint-GUID: IWrGAuu8DrsPwZMBPUm4M7SdQeMkY3j5
X-Proofpoint-ORIG-GUID: IWrGAuu8DrsPwZMBPUm4M7SdQeMkY3j5
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Nov 22, 2021, at 8:29 PM, NeilBrown <neilb@suse.de> wrote:
>=20
> Using sv_lock means we don't need to hold the service mutex over these
> updates.
>=20
> In particular,  svc_exit_thread() no longer requires synchronisation, so
> threads can exit asynchronously.
>=20
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfssvc.c |    5 ++---
> net/sunrpc/svc.c |    9 +++++++--
> 2 files changed, 9 insertions(+), 5 deletions(-)
>=20
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index fc5899502a83..e9c9fa820b17 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -55,9 +55,8 @@ static __be32			nfsd_init_request(struct svc_rqst *,
> 						struct svc_process_info *);
>=20
> /*
> - * nfsd_mutex protects nn->nfsd_serv -- both the pointer itself and the =
members
> - * of the svc_serv struct. In particular, ->sv_nrthreads but also to som=
e
> - * extent ->sv_temp_socks and ->sv_permsocks.
> + * nfsd_mutex protects nn->nfsd_serv -- both the pointer itself and some=
 members
> + * of the svc_serv struct such as ->sv_temp_socks and ->sv_permsocks.
>  *
>  * If (out side the lock) nn->nfsd_serv is non-NULL, then it must point t=
o a
>  * properly initialised 'struct svc_serv' with ->sv_nrthreads > 0 (unless
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index acddc6e12e9e..2b2042234e4b 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -523,7 +523,7 @@ EXPORT_SYMBOL_GPL(svc_shutdown_net);
>=20
> /*
>  * Destroy an RPC service. Should be called with appropriate locking to
> - * protect the sv_nrthreads, sv_permsocks and sv_tempsocks.
> + * protect sv_permsocks and sv_tempsocks.
>  */
> void
> svc_destroy(struct kref *ref)
> @@ -639,7 +639,10 @@ svc_prepare_thread(struct svc_serv *serv, struct svc=
_pool *pool, int node)
> 		return ERR_PTR(-ENOMEM);
>=20
> 	svc_get(serv);
> -	serv->sv_nrthreads++;
> +	spin_lock_bh(&serv->sv_lock);
> +	serv->sv_nrthreads +=3D 1;
> +	spin_unlock_bh(&serv->sv_lock);

atomic_t would be somewhat lighter weight. Can it be used here
instead?


> +
> 	spin_lock_bh(&pool->sp_lock);
> 	pool->sp_nrthreads++;
> 	list_add_rcu(&rqstp->rq_all, &pool->sp_all_threads);
> @@ -880,7 +883,9 @@ svc_exit_thread(struct svc_rqst *rqstp)
> 		list_del_rcu(&rqstp->rq_all);
> 	spin_unlock_bh(&pool->sp_lock);
>=20
> +	spin_lock_bh(&serv->sv_lock);
> 	serv->sv_nrthreads -=3D 1;
> +	spin_unlock_bh(&serv->sv_lock);
> 	svc_sock_update_bufs(serv);
>=20
> 	svc_rqst_free(rqstp);
>=20
>=20

--
Chuck Lever