The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Chris Mason <clm@meta.com>
Cc: Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>,
	Trond Myklebust <trondmy@gmail.com>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] nfsd: drain inflight callbacks in probe_callback_sync
Date: Tue, 19 May 2026 14:44:52 -0400	[thread overview]
Message-ID: <agyvpO10GIaWtziX@parkour.fieldses.org> (raw)
In-Reply-To: <20260519180032.1852793-4-clm@meta.com>

On Tue, May 19, 2026 at 10:49:15AM -0700, Chris Mason wrote:
> nfsd4_destroy_session() calls nfsd4_probe_callback_sync() to quiesce
> the backchannel before nfsd4_put_session_locked() drops the last
> se_ref and frees the session. nfsd4_probe_callback_sync() only
> flushes cl_callback_wq, which drains the work_struct that submits
> CB RPCs but returns before the rpciod-side completion tail
> (rpc_call_done -> rpc_release -> nfsd41_destroy_cb) has run.

nfsd4_probe_callback_sync()->nfsd4_probe_callback() sets
NFSD4_CLIENT_CB_UPDATE and calls nfsd4_run_cb(), which should ensure
work is queued that will run nfsd4_run_cb_work and call
nfsd4_process_cb_update()->setup_callback_client(), which calls
rpc_shutdown_client()--and I *think* that isn't supposed to return until
any rpc callbacks have run?  And since that's called from the workqueue,
the flush should do it.

Maybe?  I may not have thought about this in 4 years, but you did cc me,
so if I'm raving, well, you get what you deserve....

--b.

> A CB
> RPC submitted just before teardown can therefore still be executing
> on rpciod, with clp->cl_cb_session pointing at the session about to
> be freed:
> 
>     CPU 0 (DESTROY_SESSION)              CPU 1 (rpciod)
>     -----                                -----
>     nfsd4_probe_callback_sync(clp)
>       flush_workqueue(cl_callback_wq)    nfsd4_cb_sequence_done()
>                                            reads clp->cl_cb_session
>     nfsd4_put_session_locked(ses)
>       free_session(ses)
>       kfree(ses)
>                                          nfsd41_destroy_cb()
>                                            dec cl_cb_inflight
> 
> The se_ref gate in nfsd4_put_session_locked() does not close this
> window: the backchannel dispatch path does not take a se_ref via
> nfsd4_get_session_locked(), so se_ref can already be zero while a
> CB RPC is still in flight against the session.
> 
> cl_cb_inflight, added by commit 2bbfed98a4d8 ("nfsd: Fix races
> between nfsd4_cb_release() and nfsd4_shutdown_callback()"), is the
> barrier that covers the full window: nfsd4_run_cb() bumps it before
> queue_work() and nfsd41_destroy_cb() drops it from rpc_release,
> after the last cl_cb_session dereference. nfsd4_shutdown_callback()
> already calls nfsd41_cb_inflight_wait_complete() after its
> flush_workqueue() for this reason; nfsd4_probe_callback_sync() was
> not updated when cl_cb_inflight was introduced.
> 
> Fix by waiting on cl_cb_inflight in nfsd4_probe_callback_sync()
> after the workqueue flush, so every queued CB RPC has reached its
> rpc_release before the caller proceeds to free the session.
> 
> Fixes: 2bbfed98a4d82ac4 ("nfsd: Fix races between nfsd4_cb_release() and nfsd4_shutdown_callback()")
> Assisted-by: kres (claude-opus-4-7)
> Signed-off-by: Chris Mason <clm@meta.com>
> ---
>  fs/nfsd/nfs4callback.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 8af2d0cc37c2..6cf5591651e4 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1246,6 +1246,7 @@ void nfsd4_probe_callback_sync(struct nfs4_client *clp)
>  {
>  	nfsd4_probe_callback(clp);
>  	flush_workqueue(clp->cl_callback_wq);
> +	nfsd41_cb_inflight_wait_complete(clp);
>  }
>  
>  void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
> -- 
> 2.52.0
> 

  reply	other threads:[~2026-05-19 18:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 17:49 [PATCH] nfsd: drain backchannel callbacks before freeing a session Chris Mason
2026-05-19 17:49 ` [PATCH 1/2] nfsd: clear cl_cb_session on DESTROY_SESSION Chris Mason
2026-05-19 19:48   ` Jeff Layton
2026-05-19 17:49 ` [PATCH 2/2] nfsd: drain inflight callbacks in probe_callback_sync Chris Mason
2026-05-19 18:44   ` J. Bruce Fields [this message]
2026-05-19 20:01 ` [PATCH] nfsd: drain backchannel callbacks before freeing a session Jeff Layton

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=agyvpO10GIaWtziX@parkour.fieldses.org \
    --to=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=clm@meta.com \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox