All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Li Lingfeng <lilingfeng3@huawei.com>,
	chuck.lever@oracle.com, neilb@suse.de, 	okorniev@redhat.com,
	Dai.Ngo@oracle.com, tom@talpey.com, linux-nfs@vger.kernel.org,
		linux-kernel@vger.kernel.org
Cc: yukuai1@huaweicloud.com, houtao1@huawei.com, yi.zhang@huawei.com,
	 yangerkun@huawei.com, lilingfeng@huaweicloud.com
Subject: Re: [PATCH] nfsd: decrease cl_cb_inflight if fail to queue cb_work
Date: Tue, 18 Feb 2025 08:58:06 -0500	[thread overview]
Message-ID: <0ae8a05272c2eb8a503102788341e1d9c49109dd.camel@kernel.org> (raw)
In-Reply-To: <20250218135423.1487309-1-lilingfeng3@huawei.com>

On Tue, 2025-02-18 at 21:54 +0800, Li Lingfeng wrote:
> In nfsd4_run_cb, cl_cb_inflight is increased before attempting to queue
> cb_work to callback_wq. This count can be decreased in three situations:
> 1) If queuing fails in nfsd4_run_cb, the count will be decremented
> accordingly.
> 2) After cb_work is running, the count is decreased in the exception
> branch of nfsd4_run_cb_work via nfsd41_destroy_cb.
> 3) The count is decreased in the release callback of rpc_task — either
> directly calling nfsd41_cb_inflight_end in nfsd4_cb_probe_release, or
> calling nfsd41_destroy_cb in nfsd4_cb_release.
> 
> However, in nfsd4_cb_release, if the current cb_work needs to restart, the
> count will not be decreased, with the expectation that it will be reduced
> once cb_work is running.
> If queuing fails here, then the count will leak, ultimately causing the
> nfsd service to be unable to exit as shown below:
> [root@nfs_test2 ~]# cat /proc/2271/stack
> [<0>] nfsd4_shutdown_callback+0x22b/0x290
> [<0>] __destroy_client+0x3cd/0x5c0
> [<0>] nfs4_state_destroy_net+0xd2/0x330
> [<0>] nfs4_state_shutdown_net+0x2ad/0x410
> [<0>] nfsd_shutdown_net+0xb7/0x250
> [<0>] nfsd_last_thread+0x15f/0x2a0
> [<0>] nfsd_svc+0x388/0x3f0
> [<0>] write_threads+0x17e/0x2b0
> [<0>] nfsctl_transaction_write+0x91/0xf0
> [<0>] vfs_write+0x1c4/0x750
> [<0>] ksys_write+0xcb/0x170
> [<0>] do_syscall_64+0x70/0x120
> [<0>] entry_SYSCALL_64_after_hwframe+0x78/0xe2
> [root@nfs_test2 ~]#
> 
> Fix this by decreasing cl_cb_inflight if the restart fails.
> 
> Fixes: cba5f62b1830 ("nfsd: fix callback restarts")
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfsd/nfs4callback.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 484077200c5d..8a7d24efdd08 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1459,12 +1459,16 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
>  static void nfsd4_cb_release(void *calldata)
>  {
>  	struct nfsd4_callback *cb = calldata;
> +	struct nfs4_client *clp = cb->cb_clp;
> +	int queued;
>  
>  	trace_nfsd_cb_rpc_release(cb->cb_clp);
>  
> -	if (cb->cb_need_restart)
> -		nfsd4_queue_cb(cb);
> -	else
> +	if (cb->cb_need_restart) {
> +		queued = nfsd4_queue_cb(cb);
> +		if (!queued)
> +			nfsd41_cb_inflight_end(clp);
> +	} else
>  		nfsd41_destroy_cb(cb);
>  
>  }

Good catch!

Reviewed-by: Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2025-02-18 13:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-18 13:54 [PATCH] nfsd: decrease cl_cb_inflight if fail to queue cb_work Li Lingfeng
2025-02-18 13:58 ` Jeff Layton [this message]
2025-02-18 14:29   ` Jeff Layton
2025-02-18 14:31     ` Chuck Lever
2025-02-18 14:40       ` Jeff Layton
2025-02-18 15:02         ` Chuck Lever
2025-02-18 15:51           ` Jeff Layton
2025-02-18 16:01             ` Chuck Lever
2025-02-21 14:06         ` Benjamin Coddington
2025-02-21 14:37           ` Jeff Layton
2025-02-21 14:46             ` Benjamin Coddington
2025-02-21 14:58               ` Jeff Layton
2025-02-21 15:11                 ` Benjamin Coddington
2025-02-18 14:24 ` cel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0ae8a05272c2eb8a503102788341e1d9c49109dd.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=houtao1@huawei.com \
    --cc=lilingfeng3@huawei.com \
    --cc=lilingfeng@huaweicloud.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai1@huaweicloud.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.