Received-SPF: pass (google.com: domain of linux-nfs+bounces-642-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 15 Dec 2023 14:54:50 -0500
From: Chuck Lever <chuck.lever@oracle.com>
To: Dai Ngo <dai.ngo@oracle.com>
Cc: jlayton@kernel.org, linux-nfs@vger.kernel.org, linux-nfs@stwm.de
Subject: Re: [PATCH 3/3] NFSD: Fix server reboot hang problem when callback
 workqueue is stuck
Message-ID: <ZXyvCnEuV9L18JSS@tissot.1015granger.net>
References: <1702667703-17978-1-git-send-email-dai.ngo@oracle.com>
 <1702667703-17978-4-git-send-email-dai.ngo@oracle.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1702667703-17978-4-git-send-email-dai.ngo@oracle.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?nGob9SsIQN/eZvjgGKMRnuGMTFV1qUE5fJv7GgREDivm6CHcQdd8GnVDU0sX?=
 =?us-ascii?Q?NvbPaaXGNI/Ta/OANWdQ3fWksJ69jJ467wXRT+Y21wKy8jUI9c8AM95xgWqz?=
 =?us-ascii?Q?Qgef1Jf1QZBjbHp+Nn11N4FW+ZfJ+WSZ1+WEyeQotAkTDHveGAogu9EbeCeg?=
 =?us-ascii?Q?AMpEAWwEaLJkvPDEbC3bhThEbfEuPZplvoE7v6kadVBYW+hzBjhy2WuqGOI/?=
 =?us-ascii?Q?+ob6M1/B1soO3Rb3i/Ajm9pyYzRIy0NIdioUXhv1QehN/JdvtZ3O7KEBAjYk?=
 =?us-ascii?Q?bRltTV0IIzm972AVPUXv5JbCE2UEiJaGDIzI86VyghLuCPuwWTvOq1bltpnW?=
 =?us-ascii?Q?8v2dnJQbXRBTvW8ULqHVOwOdMeZABZVgww6z8+woxaUNzWlCGAZrzvaZb5d+?=
 =?us-ascii?Q?LJujiZi/nuyxp3pTkEgWw+8ZNK1XGY8rs2TW5JP3Pq+sJcBtX61MBK36Yb8o?=
 =?us-ascii?Q?zNdJqyIyBY9PrhtyovY68eOIB/1rZwjCjIP597+iuuaO534KLwvlIt8b88M8?=
 =?us-ascii?Q?4x52nfXRpdLho4dJN1QhgxMpKmqNtGsKLWTqudzCARXydhm2kTs53zhg+p7c?=
 =?us-ascii?Q?pTwfXEzLHXGKz37xlPdkpsCVW+pXAI78oQHxpJEF404YrzDxEDrGAVRvQUI+?=
 =?us-ascii?Q?wsxpYnlRjaI/RkmjN9xHqKVkcwc19UlNXeF5LRQ0cKEhDSS4Qia9NQT9K8qN?=
 =?us-ascii?Q?Lk+ANX1Kgmf8l4xAA5i4sIvXcLzUOZ/YLTBbjtc75dfNCuYe8/OoJj65wwpG?=
 =?us-ascii?Q?5FP5cmfpmsDO9OQvmTD66pHSj/Hk6A3t94t39VTXXzbox7dpqmht/rpokYwG?=
 =?us-ascii?Q?VNcXrR0JdnVY3plbvydBdITRaKQaBUR2KUCUhYi/4EtJuuiHls+eOXlPeN9N?=
 =?us-ascii?Q?sZD9xY36+s/ODcvYGPy4kDrpvgNnEPtWQLvQnpa5eKQlOe3hqAaedlGo8P8X?=
 =?us-ascii?Q?lBbjpbIdBXcAzklzTNKPp9E3QHyKD/9JmRfpNJC+HFOXGi2vSYGjGUCDyQMq?=
 =?us-ascii?Q?lW9MSzYa2yRLR2BI8XYGyip+7rYu5Y3CEWo/4H/U/t539FGP93nwOae2uQwC?=
 =?us-ascii?Q?l6rKS113qzCzrI1RD8puoebgFiahO7agixbTcq715dYN/lxUtlbB7JYX0wb5?=
 =?us-ascii?Q?QIGU9b2w+gNRPJ4QwXEMATOM2L5h3iVchQlnPjlLd1sK69tHkADExMeKHWuZ?=
 =?us-ascii?Q?lbGkqP9E7F9N4hY/xbpDpIczrlWhh0qj8t43Hy7TGDCevgySG0RNSF+4nmQK?=
 =?us-ascii?Q?xukhdzrfH8reXhYCUS8rSBkMss1J6LnMqWL9xZnDGTLLfrhCv77q68aREWk6?=
 =?us-ascii?Q?hC5kZAXNDj519YyKoylMgyABxlZhZ7sT6bky5mhXmVsvjlkiJKQq986IAq+e?=
 =?us-ascii?Q?Fg6et0qg25gw/htAfkDnEJ3EUOJiM5GowDHEFyvMob9Cxa94D2owzGoy2SOF?=
 =?us-ascii?Q?rOg3rV8ARCP3B08qYwoXQzL/Py9upaZm64MzE732SqCUojJQIgeHS12Y38si?=
 =?us-ascii?Q?mLuxBubBclZOHc+jcS+XyRTP4NAW6HAFvdIBP89zFVVs/phwyJvQKxS/mPij?=
 =?us-ascii?Q?QZk8vOiIRChqkNjDt8I1EV22//o8HgRvdA4x408sgOJN82W4ioP+z4c8T+cm?=
 =?us-ascii?Q?Ig=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	Oi2J5bW7s4tQC9mrmyvjpfH6sMH3plGR4b1ij1oDOHvveuplfHgS9/a27DIj7jBD0efo5gzwbl1mYT8p/1UZfMotjLgg0fIjpsDVyAoUbiKBqXeGe4jLPNtapLZZITvciSixNS1mJaN2m6Qu7yq9WF1AM5ZGOm3Qzr9sH23NoLwNGn1422xPIjiv23V04RCRqh5CPutf4r1SG+pQohgFtBhVl8npytjxAGPCHeHRR6C8buPGqeRfIdUjAftBtciEtDy27eyjYSOtgvm0I6O1VpmB1c5DX3CsnSxReciwTVPy8XDuVboKhWVtxph7yd1rozspptAg2oObH82AzHuAk+fPEQ7NFGDFBeukAKFcpnKK9wUwPFcD3IQsFWhPcZzYDmGEa0XpqkDIt4hW5ib8prS2VOyFRm/yTIdBWEVKbsIQyYD4gGHj7CcjQ80PdGV7sUjM2G6R1rHF5TcRg5kOTYOfVvQADiHtBB8B/gsNira8pKO1791t2d7cQaftqO+0hN+6Xw/GDqrDH2qvdaD2E4VGCotGviPd8PPMT1IZ/NZtluyqhL/fO8ovF5Pz1mXXG+vdh/5Jn58wDY9wb0IlHc/RMdSSq4fRhaHvGGZkeuY=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cfaa7cb9-5172-4506-6f04-08dbfda7b44a
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2023 19:54:53.0184
 (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: oC4njOVqJsQ3qyWOVH1k6/qK2UhIy6TDv6MacmqRzSD81XiP4OSJIUMiF5vlnEMKRgu02GBVZENQyJHhXDSXiQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6675
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-15_10,2023-12-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 spamscore=0
 malwarescore=0 mlxscore=0 mlxlogscore=999 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312150138
X-Proofpoint-GUID: Tszncdf9pgSFr0qLJqWpjNvAJDK2diSv
X-Proofpoint-ORIG-GUID: Tszncdf9pgSFr0qLJqWpjNvAJDK2diSv

On Fri, Dec 15, 2023 at 11:15:03AM -0800, Dai Ngo wrote:
> If the callback workqueue is stuck, nfsd4_deleg_getattr_conflict will
> also stuck waiting for the callback request to be executed. This causes
> the client to hang waiting for the reply of the GETATTR and also causes
> the reboot of the NFS server to hang due to the pending NFS request.
> 
> Fix by replacing wait_on_bit with wait_on_bit_timeout with 20 seconds
> time out.
> 
> Reported-by: Wolfgang Walter <linux-nfs@stwm.de>
> Fixes: 6c41d9a9bd02 ("NFSD: handle GETATTR conflict with write delegation")
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 6 +++++-
>  fs/nfsd/state.h     | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 175f3e9f5822..0cc7d4953807 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2948,6 +2948,9 @@ void nfs4_cb_getattr(struct nfs4_cb_fattr *ncf)
>  	if (test_and_set_bit(CB_GETATTR_BUSY, &ncf->ncf_cb_flags))
>  		return;
>  
> +	/* set to proper status when nfsd4_cb_getattr_done runs */
> +	ncf->ncf_cb_status = NFS4ERR_IO;
> +
>  	refcount_inc(&dp->dl_stid.sc_count);
>  	if (!nfsd4_run_cb(&ncf->ncf_getattr)) {
>  		refcount_dec(&dp->dl_stid.sc_count);
> @@ -8558,7 +8561,8 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode,
>  			nfs4_cb_getattr(&dp->dl_cb_fattr);
>  			spin_unlock(&ctx->flc_lock);
>  
> -			wait_on_bit(&ncf->ncf_cb_flags, CB_GETATTR_BUSY, TASK_INTERRUPTIBLE);
> +			wait_on_bit_timeout(&ncf->ncf_cb_flags, CB_GETATTR_BUSY,
> +				TASK_INTERRUPTIBLE, NFSD_CB_GETATTR_TIMEOUT);

I'm still thinking the timeout here should be the same (or slightly
longer than) the RPC retransmit timeout, rather than adding a new
NFSD_CB_GETATTR_TIMEOUT macro.


>  			if (ncf->ncf_cb_status) {
>  				status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));
>  				if (status != nfserr_jukebox ||
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index f96eaa8e9413..94563a6813a6 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -135,6 +135,8 @@ struct nfs4_cb_fattr {
>  /* bits for ncf_cb_flags */
>  #define	CB_GETATTR_BUSY		0
>  
> +#define	NFSD_CB_GETATTR_TIMEOUT	msecs_to_jiffies(20000) /* 20 secs */
> +
>  /*
>   * Represents a delegation stateid. The nfs4_client holds references to these
>   * and they are put when it is being destroyed or when the delegation is
> -- 
> 2.39.3
> 

-- 
Chuck Lever