Received-SPF: pass (google.com: domain of linux-nfs+bounces-2725-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 8 Apr 2024 22:09:01 -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>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: optimise recalculate_deny_mode() for a common case
Message-ID: <ZhSjPcVMJ4BBm/M6@tissot.1015granger.net>
References: <171253979210.17212.5851835299179227478@noble.neil.brown.name>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <171253979210.17212.5851835299179227478@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?vBu+JtJhXtcx6a5Q1Eunt6r3opaxYgrd1fKUhnDUzmF0bR9h1yf5HpSPASSq?=
 =?us-ascii?Q?oAbVsJyf/rSqZa7+Czcjn06FjLpSID7V4atdi7fLVQArXavPuDjMd6p5GUjk?=
 =?us-ascii?Q?t68z6Zo56hTc7p0YZQaznopR3OA9t+9P0PWydyS1rnyqDjcz50/iNnx0HC1j?=
 =?us-ascii?Q?tReveivBADARoJeOtG5T7kzgtWDLaGqZhwrtOO8bvPrNuF4R4W1Tgvv5qLPR?=
 =?us-ascii?Q?Wgm6rySQVU0Ab3RAuUF1fdkKUIsP9VmTYjI5td0U6TeB7rRtnkBne3GBmH6Q?=
 =?us-ascii?Q?TIeX7WkIGLm0txKUVzkPxTA7Xdw2VfhMVABwVzCP1ibZSQuBikaDhcM/T0+5?=
 =?us-ascii?Q?TIrRIVurhZ7G5aBChET7c2xra0nrMP/62cZy00itunperwYX45zwRjCgwutd?=
 =?us-ascii?Q?WT34H+MDRHqbx2wiaNE8TnsMbRLOf3d1ZeTCTdlGuGXLjIn7FeENJzk5NFw/?=
 =?us-ascii?Q?K6W8wtmbAPjAC3+JxH3q//2BBJ8Ts1MEiMa7mMvlPGQNRAzzJ4/YteNKI5JM?=
 =?us-ascii?Q?qRzjjkHmG0iCQtkw4gg9JnEB3kk/5H+YsWVw6usPrnkpytI42mpb7yYmwgI5?=
 =?us-ascii?Q?ePj6/gAvanGKMP/3WBJiD/6W8LNNOAb6lxsdL0zweY7hyGdnQ6DoSISZ9DaO?=
 =?us-ascii?Q?UB2gy7ACxY3pDyQjFcV+SyQgsOgCnLW3K+2Xlzsah4DTMt88rASlHfL0wOGA?=
 =?us-ascii?Q?fbEJoT0LfDVzYYfJuCNP2Bg6uJHbcuIfcVNE0wSAGossvTepc2rL3gBSRRoC?=
 =?us-ascii?Q?vTKVjUo+USDZvJ6TmoREKXWMYQVzAu9iAZAcIKx20FNQrm0KAbkuclXmKXLd?=
 =?us-ascii?Q?xzXBKWpxygRpoIy2L7Uid+DoMe9JQLYU2Ej/iJO5TZsqiBPTwX9gjD6iCvh8?=
 =?us-ascii?Q?S47hUGesC2xCNC4B04eAPAKlJ/0BOjLFX5y7TUm7BdsX1qX6BK7JxoNnsmhj?=
 =?us-ascii?Q?pML3vb3DHjz3ggUbL/XPCpPTAd+Hk6gmP09Kdn5DIDIlCj2mZ/p0kjXzjqWt?=
 =?us-ascii?Q?VPPVuPWdy/gWL3isypwj5V1X6ufP9k+PbdDtoC0QwO3+3VWZCVfGeB4QH9Jr?=
 =?us-ascii?Q?26fxKZ9kXGzcB/uuvckSN1Ga91xC9FpdzjUlWuBkNBNScq5SQYCKo+8t8M7h?=
 =?us-ascii?Q?tRukAcyncm5YsWKYoKbiyePSbCKNrRvZX/dFDd2fbSjj2tohXu4I9NXCW2Bq?=
 =?us-ascii?Q?/2XBnyIFKrgQ8hCfMCyzHS9Fyt5h8RKqKcoKJt94m20RZKR7t5u51WSgmrs/?=
 =?us-ascii?Q?kIN/O12kk/gipVA9+9iIM5URv1DxbVUhCVvhqt1jV55vepcUJvivhrfCkpPh?=
 =?us-ascii?Q?E09E/R/T8N8VwiM5hjy320rXdgHfTbcILt6opflc72DbWOxvSdIJCDtUXFPz?=
 =?us-ascii?Q?rEEqEItePkNM0A/RNEEmJvtup/4t5t7nM69nMmzmGyDauW2idNZ45mObKarH?=
 =?us-ascii?Q?62daB0G4OR9Hp8jD+nc8j/8+PIJz/htsSgPcZ/SEQxoJze14V76e7lJPAZIr?=
 =?us-ascii?Q?wrxcpjQFeRQlotJGyprOan69VkjhtAP9FmpSTvPZmhBwZB2c6UyOvZ1hz09s?=
 =?us-ascii?Q?M+CGlv6lEl1yI6nfuVEamyWR++IAXf99G+Sn5k1h5VcdWfEtCDu8W6nuBBai?=
 =?us-ascii?Q?Rw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	6pGLVIChOA+dUl3A8Tdgg+izjSM5UnFdxG6v3YgtvzcNpHD46DbOXDBxlF5NfkMUlJopSH4kGpq7RvfabPAT/skXScA4a7JBiEJVFBoMPY4OdSqT85uwed30lafObtATpVsVU/Ij6bPAAmpcm0X3qb3o9Pq3eiD8cWql0ca36FAWZ1P0irOYmLdQm0JDabU0MeU1uIa37ZbqVMlYmr4/PTsdxDM64STh2yhK4WdSN0GjFfjfIrTE2DIV2LkNUW2QQqEzsOQddIELfNBPVf37p21Xk24fj2kkOCCImCCWw11ARZsAcD+X/os+hHPARUPMH7dBHiNABqkHWlXKlq2wskj96PBcf1ifLXxZn7MbBsHwHdIfhYVlqKfB4rHXulkWAaIqcRGdToSz/G8dKQwQVUbRDLw+XrNDnPIGhn2teE3wFORwqvNHPI5YvExUYGxeR5aCnOpGcTxEF/kNDTjqkaiIO8UG8RuywDZu528Y0gEkkTRul1mTUfh0OuTM6PgbIc03R+Csip0sd38utDJZuC5qIX3JmhTmRI/Iwc0o53BGLQzQK0UKT00kkhD3Y9l4/l+j7sgzYaReFeOlkoGGCcGb1wU++Jb/juVoMtVw+mI=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2088a593-3e2a-4c9b-4c90-08dc583a074e
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 02:09:03.4780
 (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: +YwkGcLdAzplesxluYuuY1NG7DTsSWUDFhDyO+xV7lkRoovw3w870FjG7dqvPO69OBTmvxXXF+S5PUWaX6BzPg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB7439
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-08_19,2024-04-05_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0 mlxscore=0
 suspectscore=0 adultscore=0 phishscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404090011
X-Proofpoint-ORIG-GUID: x8KXdpOPMS0wRMYpO-SBknV5AbBzv8tM
X-Proofpoint-GUID: x8KXdpOPMS0wRMYpO-SBknV5AbBzv8tM

On Sun, Apr 07, 2024 at 09:29:52PM -0400, NeilBrown wrote:
> 
> recalculate_deny_mode() takes time that is linear in the number of
> stateids active on the file.
> 
> When called from
>   release_openowner -> free_ol_stateid_reaplist ->nfs4_free_ol_stateid
>   -> release_all_access
> 
> the number of times it is called is linear in the number of stateids.
> The net result is that time taken by release_openowner is quadratic in
> the number of stateids.
> 
> When the nfsd server is shut down while there are many active stateids
> this can result in a soft lockup. ("CPU stuck for 302s" seen in one case).
> 
> In many cases all the states have the same deny modes and there is no
> need to examine the entire list in recalculate_deny_mode().  In
> particular, recalculate_deny_mode() will only reduce the deny mode,
> never increase it.  So if some prefix of the list causes the original
> deny mode to be required, there is no need to examine the remainder of
> the list.
> 
> So we can improve recalculate_deny_mode() to usually run in constant
> time, so release_openowner will typically be only linear in the number
> of states.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

Applied to nfsd-next. Thanks!


> ---
>  fs/nfsd/nfs4state.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 1824a30e7dd4..a46f5230bc9b 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1409,11 +1409,16 @@ static void
>  recalculate_deny_mode(struct nfs4_file *fp)
>  {
>  	struct nfs4_ol_stateid *stp;
> +	u32 old_deny;
>  
>  	spin_lock(&fp->fi_lock);
> +	old_deny = fp->fi_share_deny;
>  	fp->fi_share_deny = 0;
> -	list_for_each_entry(stp, &fp->fi_stateids, st_perfile)
> +	list_for_each_entry(stp, &fp->fi_stateids, st_perfile) {
>  		fp->fi_share_deny |= bmap_to_share_mode(stp->st_deny_bmap);
> +		if (fp->fi_share_deny == old_deny)
> +			break;
> +	}
>  	spin_unlock(&fp->fi_lock);
>  }
>  
> -- 
> 2.44.0
> 

-- 
Chuck Lever