Message-ID: <458a70c7-f5ba-4aea-158e-19f5a44d7a62@oracle.com>
Date:   Sat, 4 Feb 2023 12:50:34 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: [PATCH] nfsd: fix courtesy client with deny mode handling in
 nfs4_upgrade_open
Content-Language: en-US
To:     Jeff Layton <jlayton@kernel.org>, chuck.lever@oracle.com
Cc:     linux-nfs@vger.kernel.org,
        =?UTF-8?B?5by15pm66Ku6?= <cc85nod@gmail.com>
References: <20230203181834.58634-1-jlayton@kernel.org>
From:   dai.ngo@oracle.com
In-Reply-To: <20230203181834.58634-1-jlayton@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eEFKTHlJQndhUXF2TjlGSVFRZHlvcERWbTZEVGtPWFFKVU9ZclFCYkJ1bEZ4?=
 =?utf-8?B?WjUxZGJXejZpYkR4TkFIOGtNeDNDUEV3TjY4QmhMUDBxUUpQRnhaN2NmN2Nq?=
 =?utf-8?B?RTBHR1pJOXFVN1V3RUdlUUp0cU5HWk5ZQUFmUisxV29vVmJJb3NSTmVCOXlz?=
 =?utf-8?B?Z2JTSWdTQ3lRR1dNSExLcU9jbXRhd01ObFBxS2dyRU5LSjRPdG1qa3hUVkZ2?=
 =?utf-8?B?bFpiR0ZWU3VIeC9ORG4vZWlwZ0owazlkN0JJUm5NdVhFSDNRQm02Z2pDWFBB?=
 =?utf-8?B?NnVHdkJiRUtOdmdSN29Ea2RHeWpMeHNmWnA4UjVGcEpuVUxMTHRjZUp6V2tU?=
 =?utf-8?B?TzNxZEpITmwzbXl2VEdhTnNvY0pHeXh2RmVaR0RDMm9pbWpIUUJUanJjck5B?=
 =?utf-8?B?V1krLzlHbUZuTTJiSlRsZFRZSEhiaFV2TDJ6VG1JaGdMVWgvTkI5SndWOGVZ?=
 =?utf-8?B?RFRjMFpsSDBvV2dsYUNobXArc1RlZVhMNUVwV1h4TzVqQi9TbkV1SUVGVy8x?=
 =?utf-8?B?RFgxaWIwVzlXL3BaOWpYZkJHQ21DL3M4dUNZbTVKZTFPaURoQzY0MmFscDFl?=
 =?utf-8?B?M1E5a2F2MjBZNnZ0c1hKeXNCODMycXVxN3VmTTJrZzgrRmZFVVNoaWowU3Nh?=
 =?utf-8?B?QTRSQlJpQTdvam9YTWJ5aDIvTi8xVGxVdWxoVlVsdnY0UTUweUNMb0JCN3ly?=
 =?utf-8?B?SzJHQzJRUVdIb2s0SHB3dDRmam1aeGxDcU5nczQzdnpCeXExeTJQVFpUTUsz?=
 =?utf-8?B?U3dvdkQ0NUhGL0MrQ0VQWmtGL1Q1R3FKcXpINzcxekhYS0hiVkRMUHArWURK?=
 =?utf-8?B?SHNrRk9jRGYzZ2ZpMWVXQzg4M2N5ZnBIdjlyRm04aENCSnJMYWZRTW53SWZx?=
 =?utf-8?B?eW1pcEwyUjFQejJuSXl0dCtIR0xFbkZVYjFlM1Jqa01aNFA0Mm80YnhmK1V3?=
 =?utf-8?B?aE1mTTIvNVl2cTR5Y1RGWDFJTFdZVXYzTExYVTR3ci91WWRrNlZBemxCQTRj?=
 =?utf-8?B?SksrbVhoczJ0OFZ3WVc5c0J6djNuL25DV1F1ZjBoUldxbERUTkk5bE40THdt?=
 =?utf-8?B?cjNMdjhZU05vQTFGQUsxNnVZUkR6dDZOK09XSnVjYzNlbXVNYWphY0NBZkUz?=
 =?utf-8?B?UkxFU21xcXNQNUdIaDlrMWcwakhESDJDOHY3M28weEEreWxKS2JlSW4yVUpw?=
 =?utf-8?B?MmJQaUlDb2ZlN3dkTlJ6OW5oWSsxTHRMemx2clFWV2JnSlJua0RUd01SVlRM?=
 =?utf-8?B?d040L3FPRGZwQThSZVZ2LzEvS1dPbGlXeXdZM2Z6aVErdUxhUWNzK01mQlRp?=
 =?utf-8?B?K3gwN1BOMmZWZnR2Z3h3TTl6NzRuYUhsTDkwOXlVU3BWVjBWNVBtWGlFVDh4?=
 =?utf-8?B?U1FoK1k2ZFJWTmhSTFFMMDhTSDl2UElISWdMdU8zR0s2QXczcFhyVmJIcWk5?=
 =?utf-8?B?M3JOMmdVVTRHdm5BYzFuMXdkekF1U1crQ2hpWWkvUXJPdFB5VWZlT0s4VHRS?=
 =?utf-8?B?aEJIOUV3RkFiKzI4QTBIL2NVcFhmN3c1RHpqL1czc3dSb0VoS1lneUw3M1Q0?=
 =?utf-8?B?RHZCMmpsTWU5M21sNDBLKzdKUXhYVnN4MU9ia2tuZW5YdVFDeCtUODVJVEtz?=
 =?utf-8?B?WUFWRTVsb1R5ZlVzbGVndlhRNUhWMENUajBhQnBiOElwQmFVTFJVSjBYM0Uy?=
 =?utf-8?B?dFM2L09lUUcxNjZJMVJsN0FSajc3Nm5UTzIxYis5WjZCYm9EcHpoeUFuSnhw?=
 =?utf-8?B?K3NpNGJ0a0V4MjRFd0F0cHVWMEJhQmZCVGxJU2ZpL3N5MVkva1Rmd3pSZXlj?=
 =?utf-8?B?cW5vRHdZK0g1NjRzc1JGZCsxQkVRYkRaS0J2Y3FPUjdzSGlZeGNsMlowVStJ?=
 =?utf-8?B?UGwzNDkvemREUWV2VlhOSmRJVGtheFpSOWpZY1pVTGtCVnNxaGxwalFNTDEz?=
 =?utf-8?B?bll3MGVGTmZmTDZTNEpCOVpIMjJaWjhtcEJKM2ZrN1daRVdhOUIwMkJ1Z0hF?=
 =?utf-8?B?NW9YMTZGVk5BbkdlOWNLQWtFT2FBNWZSdlBrZkNjMU5qYXdFcmxRUXJBY2pp?=
 =?utf-8?B?amdCbGhIZ01OL3F5d1VUR0phZnNETWFod0JHWjRIczBzN1V1WWJTQXJoWkdT?=
 =?utf-8?Q?G88chrl1d9DvCn0PSrJECnG4/?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: nTJcP0pj57H+ytcn9Yr2xxM9rtcWuitzK1E/4Gvy6f1nzekV9Qbb5UewmJJ+X+mxhOI3pP2pqUB00ERGo4FpiOodO+qxCP41zr9GOX5ahlYNk+E4yeYjF3wIYEO3UeSy02I2zzFz+x65fH1sIwY+bHsXLKWU17HuISb3SAnPwcAY7otMhVvQS6U0kEl4PrPbZkzxvo5Ap+6NlfTwW8J9amvw6S0bo0YGwbRxeQAsklH+zSfUKwX1h20COc53Q8vQuDN6VONNcUggIMaluWp8lfpvr7ZoJ9baEhCQ18ZIa7C6ljvRuA+EB73JhNLUY26sAdAlKe+7R8OWOkeiHW025CXtaw5/uEd5Mr+27mRmMjbp8AJj05JFcxHvft+k3t77BXDDJho1EhQlu6b+If2nmZAAnNhqTgW4Bh+/ATPJKs2u0ldgETap+w74i2Tu4bhOvcsDparMDQM70DQM8yUpNLuUGaHhleqoPLpq8P8H71Ad0lT/oVZioiTOu0LJwsXb/pLl3RTT+ERDdNMuhKlO1PdAlEt6GaVZbrj9Z8+ptlIrsOx00dXf14tV0SWl7vAtUdTOthA+quxP2n1sGk+fLnByUjQ1wIJ1bUEVhtU/9oAZNRUiS7/LmJt/nM2lpt0QLp3X7RsxLNYMdBodO5ZFpC8b5e0bCobOg1Af3/PeQF7Vayhgdk3sXxa4kC0vkcbQ98b1rwBdlkDMRSH4KkHO9NsY+KoEbNCQ+gH3BzCE2a79nsc4sxtk+xMCUB3Z9sc3+3NiLoo4XC7aSemoAo1dAxTHzjUXdCMuRkmvREadKHjYSMgiT5Isolb77Al7jkDh
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4f4c7ea2-2943-4940-609c-08db06f17801
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4257.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Feb 2023 20:50:37.5016
 (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: A9BZ3rSzEmE8gNZrjWXTmSMgf+HE5VkQZ+DWexOaknYQCKPYs2T3Qt/TKmEWL6++NenY1H+JUCIsr4mxQIZ6pQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB6372
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-02-04_11,2023-02-03_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxlogscore=999
 adultscore=0 phishscore=0 bulkscore=0 malwarescore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2302040188
X-Proofpoint-ORIG-GUID: qydoobdxjyavcERP5Bvx32lzRdo3u8Li
X-Proofpoint-GUID: qydoobdxjyavcERP5Bvx32lzRdo3u8Li
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


On 2/3/23 10:18 AM, Jeff Layton wrote:
> The nested if statements here make no sense, as you can never reach
> "else" branch in the nested statement. Fix the error handling for
> when there is a courtesy client that holds a conflicting deny mode.
>
> Fixes: 3d69427151806 (NFSD: add support for share reservation conflict to courteous server)
> Reported-by: 張智諺 <cc85nod@gmail.com>
> Cc: Dai Ngo <dai.ngo@oracle.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/nfsd/nfs4state.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c39e43742dd6..af22dfdc6fcc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5282,16 +5282,17 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp,
>   	/* test and set deny mode */
>   	spin_lock(&fp->fi_lock);
>   	status = nfs4_file_check_deny(fp, open->op_share_deny);
> -	if (status == nfs_ok) {
> -		if (status != nfserr_share_denied) {
> -			set_deny(open->op_share_deny, stp);
> -			fp->fi_share_deny |=
> -				(open->op_share_deny & NFS4_SHARE_DENY_BOTH);
> -		} else {
> -			if (nfs4_resolve_deny_conflicts_locked(fp, false,
> -					stp, open->op_share_deny, false))
> -				status = nfserr_jukebox;
> -		}
> +	switch (status) {
> +	case nfs_ok:
> +		set_deny(open->op_share_deny, stp);
> +		fp->fi_share_deny |=
> +			(open->op_share_deny & NFS4_SHARE_DENY_BOTH);
> +		break;
> +	case nfserr_share_denied:
> +		if (nfs4_resolve_deny_conflicts_locked(fp, false,
> +				stp, open->op_share_deny, false))
> +			status = nfserr_jukebox;
> +		break;
>   	}
>   	spin_unlock(&fp->fi_lock);

Reviewed-by: Dai Ngo <dai.ngo@oracle.com>

>