Re: [PATCH v6 5/6] nfsd: count NFSv4 callback operations per netns

[Chuck Lever]

On Fri, Jun 19, 2026, at 11:26 AM, Jeff Layton wrote:
> The NFS server tracks per-operation call counts for the forward channel
> (proc4ops) but keeps no statistics for the NFSv4 backchannel (callback)
> operations it sends to clients.
>
> Add a per-netns array of percpu counters for callback operations, indexed
> by RFC 8881 callback opcode (OP_CB_GETATTR..OP_CB_OFFLOAD), and bump the
> relevant counter in nfsd4_run_cb(), which is hit exactly once per callback
> that is actually queued.

> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 71dcb448fa0a..b171257da6ba 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1921,12 +1921,32 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
>  bool nfsd4_run_cb(struct nfsd4_callback *cb)
>  {
>  	struct nfs4_client *clp = cb->cb_clp;
> +	struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> +	const struct nfsd4_callback_ops *ops = cb->cb_ops;
> +	u32 minorversion = clp->cl_minorversion;
>  	bool queued;
>
> +	/*
> +	 * Snapshot the opcode, minorversion, and the per-net pointer before
> +	 * queuing: once nfsd4_queue_cb() has queued the work, the callback (and
> +	 * the object that embeds it) may be processed and freed concurrently, so
> +	 * neither cb nor clp can be dereferenced afterward.
> +	 */

The comment says the opcode is snapshotted before queuing, but the code
captures the cb_ops pointer and reads ops->opcode after nfsd4_queue_cb()
returns:

	if (ops)
		nfsd_stats_cb_op_inc(nn, ops->opcode);

That read is safe because every cb_ops points at a file-scope
static const nfsd4_callback_ops (nfsd4_cb_recall_ops,
nfsd4_cb_getattr_ops, and so on), which outlives cb, unlike cb and clp
themselves.  Would the comment read more accurately as snapshotting the
cb_ops pointer rather than the opcode, perhaps noting that ops stays
valid afterward because it points at static data?

>  	nfsd41_cb_inflight_begin(clp);
>  	queued = nfsd4_queue_cb(cb);
> -	if (!queued)
> +	if (queued) {
> +		if (ops)
> +			nfsd_stats_cb_op_inc(nn, ops->opcode);
> +		/*
> +		 * Minorversion > 0 callbacks prepend a CB_SEQUENCE op (see
> +		 * encode_cb_sequence4args()); count it like the forechannel
> +		 * counts SEQUENCE, so it isn't perpetually reported as zero.
> +		 */
> +		if (minorversion > 0)
> +			nfsd_stats_cb_op_inc(nn, OP_CB_SEQUENCE);

Can this inflate the OP_CB_SEQUENCE count for callbacks that never send
a CB_SEQUENCE?  The increment fires whenever minorversion > 0, including
the ops == NULL case.  The only callback with cb_ops == NULL is the
null probe:

  nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);

which nfsd4_probe_callback() and nfsd4_shutdown_callback() queue via
nfsd4_run_cb(&clp->cl_cb_null).  For a 4.1+ client, nfsd4_run_cb_work()
drops that work item without sending any RPC:

	/*
	 * Don't send probe messages for 4.1 or later.
	 */
	if (!cb->cb_ops && clp->cl_minorversion) {
		nfsd4_mark_cb_state(clp, NFSD4_CB_UP);
		nfsd41_destroy_cb(cb);
		return;
	}

So callback-channel probes, backchannel changes, and client teardown
each bump OP_CB_SEQUENCE for a minorversion > 0 client even though no
CB_SEQUENCE (and no callback at all) goes on the wire.

Would moving the CB_SEQUENCE increment inside the if (ops) block avoid
this?  A real callback on a 4.1+ client always prepends CB_SEQUENCE, and
the null probe carries cb_ops == NULL, so:

	if (queued) {
		if (ops) {
			nfsd_stats_cb_op_inc(nn, ops->opcode);
			if (minorversion > 0)
				nfsd_stats_cb_op_inc(nn, OP_CB_SEQUENCE);
		}
	} else {
		nfsd41_cb_inflight_end(clp);
	}

> +	} else {
>  		nfsd41_cb_inflight_end(clp);
> +	}
>  	return queued;
>  }


-- 
Chuck Lever