Received-SPF: pass (google.com: domain of linux-nfs+bounces-2916-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 22 Apr 2024 09:34:37 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: NeilBrown <neilb@suse.de>
Cc: Jeff Layton <jlayton@kernel.org>, Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        Petr Vorel <pvorel@suse.cz>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] nfsd: don't fail OP_SETCLIENTID when there are lots of
 clients.
Message-ID: <ZiZnbV+htcvGuGQl@tissot.1015granger.net>
References: <171375175915.7600.6526208866216039031@noble.neil.brown.name>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <171375175915.7600.6526208866216039031@noble.neil.brown.name>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?etd7/yuW8TemS+C3IfEi9aM+uREZLI1xfdDkEp9DEoU9ERmnrNnglO7kqJNL?=
 =?us-ascii?Q?CtnP3AQeD9jtL7Lu/8TINeNIJENb7f1Ari72cZTBCpkJKkFyE1HrQ4tV4Tuf?=
 =?us-ascii?Q?dWab6n+nM6t/jm+PkP9wW5EJdyPNer49Lyd78DdVx7CWy9RWCmh2rZYjqgaE?=
 =?us-ascii?Q?ozGDmqtPT9hsmdNO+nyfApd5WIsuiVLlCT+MRJB8PK7F6SywpWSSn7qDTuI9?=
 =?us-ascii?Q?guu8OoATn4uoTdwVEP2U7s3hYDJ3gGFvCpwkKi1KR9st+8lnsqN3eVJ87Hge?=
 =?us-ascii?Q?ctiIAFRhqQmqXdI23WNqxVXIvYLD5MKbh8kB7nEvzTNB8RPBuyzRfO5mvFdU?=
 =?us-ascii?Q?TOWjb10p1b1ghkzv7c5eIEqGMlIon1jGqxjMGsl+e3vakqiizsEfNT3s0iNA?=
 =?us-ascii?Q?YdAWo/+elIMLUFbfPBegJ8T7VqUsLJMSpM6umAfi6iUmMJnIfopZflYv7lmI?=
 =?us-ascii?Q?Bg2hL1dntnKTiIh91QjeoOA6ommMF2ywQeG7b2oqJLWOH1QK16H3wqicxf+t?=
 =?us-ascii?Q?XgLy2NwkReC0q1CPfInKyqP8beHVjhuAOubOJyaq27I26u+oVDf5YEbM3yJ9?=
 =?us-ascii?Q?9aqXoer/5TgO1cvFGIznNPTwRCjW0dkKu+Gpjl3pqT90cFwemPChMhvyXip3?=
 =?us-ascii?Q?+rwIO1XvGt6b1h7HdAnzvzItnHjMJYauirIrQ4hxZEIN7F/7CRJy1C59BG14?=
 =?us-ascii?Q?r7Mnrd1sqkJNzflilauLHNNL+es585gLKOAsZP4Bn590TCS4e7rlMQ03Zykx?=
 =?us-ascii?Q?Vl68zvH+gR5v7Tcls5zJRWbX/gEi1Q0vXuzLy7gq878BIITV6EUge1p6euEM?=
 =?us-ascii?Q?ZDRVGbvmYuGNRx9Znikdmxqd1ntNYLA6pToWrBLwvxEzeoC5lyXDy8kbR8CK?=
 =?us-ascii?Q?iWUX/WG43Z4XBjLmJLFwKrLsanupAsmZVLmdABSkq5SoFpLrgamZfA/M0TzE?=
 =?us-ascii?Q?2OnhFPl0MhOoycvWh0ZZ7yMbb+2w79/HfOEji8efy0phULgunQvCRl3br72I?=
 =?us-ascii?Q?Tt1zA28xALiYz5G7y1pwRXV5JnLknNXtT+MmKfG79+E1IHhItdGnD4gSgzkr?=
 =?us-ascii?Q?1sTpnHT3pCcMTJPOi9GbahM+5Z9QQOV3wuOWaulANNcxtGrqjp81Vh/SyNEl?=
 =?us-ascii?Q?JgKS6G8WZzuZOipD6xFTuzH2Uh7J9f/FCeJFdIxoTBsFFUtJaKz2B9EZck87?=
 =?us-ascii?Q?c64KEi3xlSULqtL4borYUYzoS4D12T82nL6PYIn9V/7igy8AOWaj/v0D83Tz?=
 =?us-ascii?Q?6envaZbpr/LxQ5SpGIc8PFngqcxCxd5SGlZA1AlXV2N7cSDvGCVV+t4CWJuN?=
 =?us-ascii?Q?NxNqM55bd9o8in0jn4uWYFawfxnI37LpCn2KltUOuI9TdeWcQ/WVCtAT1MG5?=
 =?us-ascii?Q?1G8HCQmvQFwl5wSysMhAIEHp74buxsJ3GCqx6FyWc6jKseppZda0+lptiuTS?=
 =?us-ascii?Q?S9ooNxAJTLjjyx/moLAhTr2H0LvQvrKL/OpSAbThkCOliYQ9p6ajqHXxa+N3?=
 =?us-ascii?Q?Xl8e46aA64xyFCdC+0yyxkGOYAqwxrZPPf6MuKG1vw4jt/mtu8LYvLDzzlzb?=
 =?us-ascii?Q?jIKGQ3pSria7YSEl3+p64PLaNVbfWy9y1s7EZfdooajglnoc3tjeZ7I4uHm6?=
 =?us-ascii?Q?bA=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	UctqDFOOxWEyT5QLs7Nt7lSqdqmJiZNuh3hmcXxoMDSLumRkzd92X7vmmqM8/tBHfckbFzYvssD+wkr37PsbEdO31Ff58tylvCzg2/62/9QD4CRcmF4u4oFztD40VOcRJtZyWcI1N/hLOryfkoLF1OvSINsmrYg2igg2F0pOchGQexfg/s8GWzwt57DLezTXb5CA7MQ8HQ3Y2PCLRgpvvyODiimo7LGaLqOW6y3LrS7s3HvglbL1SXsVkuutC0rBN/dm032O9hRVMXjcjmdTJ+7t0cqKOjx6zcv448RSYrYi8sYI765l1UifLzUdjUBsbFx9O+OEOWetg8qWrwJVng0S+vhfYm6zW3mmPzbzoWQq3YlDpHy9AzFIpMftk7LYp96czfGTVrq+bZm/KWd47TPVDvmQGuE4OptMe+M19LQEqPiGcDAn5ccZOcAvl+smF9RN4lhMVw2HQkzMEKDKlRt4Esvx8thpGfYmoXir3Wd4BykRRJXMf0ct68m9Avm/+AwFzfv+TiS+gwhqE4jov5VzrnquPWbdwYllKj7XCnxKB24sqlYFcp57j41Y0F5ZEeg7zaMEgHUg4CWu2aREn2bPg3BTDCSD7FoeaQIpWyo=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e5e849de-d364-4680-dc36-08dc62d0f66d
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2024 13:34:40.8409
 (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: 44TtBuVfIT8uzSY/ND7cEzBIgkvzUyKm8DTv5kIVXIbKbzGIrI3bP8Fq9Tr46wgEDkpPtDCbAzj8Bx4WFsaufw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4223
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-22_09,2024-04-22_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 mlxscore=0
 mlxlogscore=954 bulkscore=0 malwarescore=0 spamscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404220059
X-Proofpoint-ORIG-GUID: 7j6gR9KAuxO-ZBhgJvQVeFfedl9bZB3L
X-Proofpoint-GUID: 7j6gR9KAuxO-ZBhgJvQVeFfedl9bZB3L

On Mon, Apr 22, 2024 at 12:09:19PM +1000, NeilBrown wrote:
> The calculation of how many clients the nfs server can manage is only an
> heuristic.  Triggering the laundromat to clean up old clients when we
> have more than the heuristic limit is valid, but refusing to create new
> clients is not.  Client creation should only fail if there really isn't
> enough memory available.
> 
> This is not known to have caused a problem is production use, but
> testing of lots of clients reports an error and it is not clear that
> this error is justified.

It is justified, see 4271c2c08875 ("NFSD: limit the number of v4
clients to 1024 per 1GB of system memory"). In cases like these,
the recourse is to add more memory to the test system.

However, that commit claims that the client is told to retry; I
don't expect client creation to fail outright. Can you describe the
failure mode you see?

Meanwhile, we need to have broader and more regular testing of NFSD
on memory-starved systems. That's a long-term project.


> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/nfs4state.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index daf83823ba48..8a40bb6a4a67 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2223,10 +2223,9 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name,
>  	struct nfs4_client *clp;
>  	int i;
>  
> -	if (atomic_read(&nn->nfs4_client_count) >= nn->nfs4_max_clients) {
> +	if (atomic_read(&nn->nfs4_client_count) >= nn->nfs4_max_clients)
>  		mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
> -		return NULL;
> -	}
> +
>  	clp = kmem_cache_zalloc(client_slab, GFP_KERNEL);
>  	if (clp == NULL)
>  		return NULL;
> -- 
> 2.44.0
> 
> 

-- 
Chuck Lever