On 26 Jan 2024, at 9:12, Chuck Lever wrote:
> On Thu, Jan 25, 2024 at 04:49:17PM -0500, Benjamin Coddington wrote:
>> On 25 Jan 2024, at 11:29, Chuck Lever wrote:
>>
>>> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>>
>>> Help observe the flow of callback operations.
>>>
>>> bc_shutdown() records exactly when the backchannel RPC client is
>>> destroyed and cl_cb_client is replaced with NULL.
>>>
>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>> ---
>>> fs/nfsd/nfs4callback.c | 7 +++++++
>>> fs/nfsd/trace.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>>> include/trace/misc/nfs.h | 34 ++++++++++++++++++++++++++++++++++
>>> 3 files changed, 83 insertions(+)
>>>
>>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>>> index 1c85426830b1..4d5a6370b92c 100644
>>> --- a/fs/nfsd/nfs4callback.c
>>> +++ b/fs/nfsd/nfs4callback.c
>>> @@ -887,6 +887,7 @@ static struct workqueue_struct *callback_wq;
>>>
>>> static bool nfsd4_queue_cb(struct nfsd4_callback *cb)
>>> {
>>> + trace_nfsd_cb_queue(cb->cb_clp, cb);
>>> return queue_work(callback_wq, &cb->cb_work);
>>> }
>>>
>>> @@ -1106,6 +1107,7 @@ static void nfsd41_destroy_cb(struct nfsd4_callback *cb)
>>> {
>>> struct nfs4_client *clp = cb->cb_clp;
>>>
>>> + trace_nfsd_cb_destroy(clp, cb);
>>> nfsd41_cb_release_slot(cb);
>>> if (cb->cb_ops && cb->cb_ops->release)
>>> cb->cb_ops->release(cb);
>>> @@ -1220,6 +1222,7 @@ static bool nfsd4_cb_sequence_done(struct rpc_task *task, struct nfsd4_callback
>>> goto out;
>>> need_restart:
>>> if (!test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags)) {
>>> + trace_nfsd_cb_restart(clp, cb);
>>> task->tk_status = 0;
>>> cb->cb_need_restart = true;
>>
>> I think you want to call the tracepoint /after/ setting cb_need_restart here..
>
> Maybe?
>
> The trace point currently captures the value of cb_need_restart
> before this logic overwrites it. Is that beneficial for
> troubleshooting?
Got it, yes its better the way you have it - and the tracepoint name makes
clear we want to restart.
Ben
