[PATCH v2 0/5] Implement referring call lists for CB_OFFLOAD

[PATCH v2 0/5] Implement referring call lists for CB_OFFLOAD

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

I've built a naive proof-of-concept of the csa_referring_call_list
argument of the CB_SEQUENCE operation, and hooked it up for the
CB_OFFLOAD callback operation.

This has been pushed to my kernel.org "fix-async-copy" branch for
folks to play around with.

I've done some basic testing with a server that ensures the
CB_OFFLOAD callback is sent before the COPY reply, while running a
network capture. Operation appears correct, Wireshark is happy
with the construction of the XDR, and the CB_SEQUENCE arguments
match the SEQUENCE operation in the COPY COMPOUND.

I'd like to include this series in nfsd-testing.

Changes since RFC:
- Add a field to struct nfsd4_slot that records its table index
- Include a few additional COPY-related fixes
- Some operational testing has been done

Chuck Lever (5):
  NFSD: OFFLOAD_CANCEL should mark an async COPY as completed
  NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY
  NFSD: Implement CB_SEQUENCE referring call lists
  NFSD: Record each NFSv4 call's session slot index
  NFSD: Use a referring call list for CB_OFFLOAD

 fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
 fs/nfsd/nfs4proc.c     |  16 ++++-
 fs/nfsd/nfs4state.c    |  38 ++++++------
 fs/nfsd/state.h        |  23 +++++++
 fs/nfsd/xdr4.h         |   4 ++
 fs/nfsd/xdr4cb.h       |   5 +-
 6 files changed, 193 insertions(+), 25 deletions(-)

-- 
2.47.0

[PATCH v2 1/5] NFSD: OFFLOAD_CANCEL should mark an async COPY as completed

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

Update the status of an async COPY operation when it has been
stopped. OFFLOAD_STATUS needs to indicate that the COPY is no longer
running.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4proc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f6e06c779d09..9a0e68aa246f 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1379,8 +1379,11 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
 static void nfsd4_stop_copy(struct nfsd4_copy *copy)
 {
 	trace_nfsd_copy_async_cancel(copy);
-	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags))
+	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags)) {
 		kthread_stop(copy->copy_task);
+		copy->nfserr = nfs_ok;
+		set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
+	}
 	nfs4_put_copy(copy);
 }
 
-- 
2.47.0

[PATCH v2 2/5] NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

Try not to prolong the wait for completion of a COPY or COPY_NOTIFY
operation.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9a0e68aa246f..3431b695882d 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1712,7 +1712,7 @@ static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
 	switch (task->tk_status) {
 	case -NFS4ERR_DELAY:
 		if (cbo->co_retries--) {
-			rpc_delay(task, 1 * HZ);
+			rpc_delay(task, HZ / 5);
 			return 0;
 		}
 	}
-- 
2.47.0

[PATCH v2 3/5] NFSD: Implement CB_SEQUENCE referring call lists

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

We have yet to implement a mechanism in NFSD for resolving races
between a server's reply and a related callback operation. For
example, a CB_OFFLOAD callback can race with the matching COPY
response. The client will not recognize the copy state ID in the
CB_OFFLOAD callback until the COPY response arrives.

Trond adds:
> It is also needed for the same kind of race with delegation
> recalls, layout recalls, CB_NOTIFY_DEVICEID and would also be
> helpful (although not as strongly required) for CB_NOTIFY_LOCK.

RFC 8881 Section 20.9.3 describes referring call lists this way:
> The csa_referring_call_lists array is the list of COMPOUND
> requests, identified by session ID, slot ID, and sequence ID.
> These are requests that the client previously sent to the server.
> These previous requests created state that some operation(s) in
> the same CB_COMPOUND as the csa_referring_call_lists are
> identifying. A session ID is included because leased state is tied
> to a client ID, and a client ID can have multiple sessions. See
> Section 2.10.6.3.

Introduce the XDR infrastructure for populating the
csa_referring_call_lists argument of CB_SEQUENCE. Subsequent patches
will put the referring call list to use.

Note that cb_sequence_enc_sz estimates that only zero or one rcl is
included in each CB_SEQUENCE, but the new infrastructure can
manage any number of referring calls.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
 fs/nfsd/state.h        |  22 +++++++
 fs/nfsd/xdr4cb.h       |   5 +-
 3 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 484077200c5d..f1fffff69330 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -419,6 +419,29 @@ static u32 highest_slotid(struct nfsd4_session *ses)
 	return idx;
 }
 
+static void
+encode_referring_call4(struct xdr_stream *xdr,
+		       const struct nfsd4_referring_call *rc)
+{
+	encode_uint32(xdr, rc->rc_sequenceid);
+	encode_uint32(xdr, rc->rc_slotid);
+}
+
+static void
+encode_referring_call_list4(struct xdr_stream *xdr,
+			    const struct nfsd4_referring_call_list *rcl)
+{
+	struct nfsd4_referring_call *rc;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN);
+	xdr_encode_opaque_fixed(p, rcl->rcl_sessionid.data,
+					NFS4_MAX_SESSIONID_LEN);
+	encode_uint32(xdr, rcl->__nr_referring_calls);
+	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list)
+		encode_referring_call4(xdr, rc);
+}
+
 /*
  * CB_SEQUENCE4args
  *
@@ -436,6 +459,7 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
 				    struct nfs4_cb_compound_hdr *hdr)
 {
 	struct nfsd4_session *session = cb->cb_clp->cl_cb_session;
+	struct nfsd4_referring_call_list *rcl;
 	__be32 *p;
 
 	if (hdr->minorversion == 0)
@@ -444,12 +468,16 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
 	encode_nfs_cb_opnum4(xdr, OP_CB_SEQUENCE);
 	encode_sessionid4(xdr, session);
 
-	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4 + 4);
+	p = xdr_reserve_space(xdr, XDR_UNIT * 4);
 	*p++ = cpu_to_be32(session->se_cb_seq_nr[cb->cb_held_slot]);	/* csa_sequenceid */
 	*p++ = cpu_to_be32(cb->cb_held_slot);		/* csa_slotid */
 	*p++ = cpu_to_be32(highest_slotid(session)); /* csa_highest_slotid */
 	*p++ = xdr_zero;			/* csa_cachethis */
-	xdr_encode_empty_array(p);		/* csa_referring_call_lists */
+
+	/* csa_referring_call_lists */
+	encode_uint32(xdr, cb->cb_nr_referring_call_list);
+	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list)
+		encode_referring_call_list4(xdr, rcl);
 
 	hdr->nops++;
 }
@@ -1306,10 +1334,102 @@ static void nfsd41_destroy_cb(struct nfsd4_callback *cb)
 	nfsd41_cb_inflight_end(clp);
 }
 
-/*
- * TODO: cb_sequence should support referring call lists, cachethis,
- * and mark callback channel down on communication errors.
+/**
+ * nfsd41_cb_referring_call - add a referring call to a callback operation
+ * @cb: context of callback to add the rc to
+ * @sessionid: referring call's session ID
+ * @slotid: referring call's session slot index
+ * @seqno: referring call's slot sequence number
+ *
+ * Caller serializes access to @cb.
+ *
+ * NB: If memory allocation fails, the referring call is not added.
  */
+void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
+			      struct nfs4_sessionid *sessionid,
+			      u32 slotid, u32 seqno)
+{
+	struct nfsd4_referring_call_list *rcl;
+	struct nfsd4_referring_call *rc;
+	bool found;
+
+	might_sleep();
+
+	found = false;
+	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list) {
+		if (!memcmp(rcl->rcl_sessionid.data, sessionid->data,
+			   NFS4_MAX_SESSIONID_LEN)) {
+			found = true;
+			break;
+		}
+	}
+	if (!found) {
+		rcl = kmalloc(sizeof(*rcl), GFP_KERNEL);
+		if (!rcl)
+			return;
+		memcpy(rcl->rcl_sessionid.data, sessionid->data,
+		       NFS4_MAX_SESSIONID_LEN);
+		rcl->__nr_referring_calls = 0;
+		INIT_LIST_HEAD(&rcl->rcl_referring_calls);
+		list_add(&rcl->__list, &cb->cb_referring_call_list);
+		cb->cb_nr_referring_call_list++;
+	}
+
+	found = false;
+	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list) {
+		if (rc->rc_sequenceid == seqno && rc->rc_slotid == slotid) {
+			found = true;
+			break;
+		}
+	}
+	if (!found) {
+		rc = kmalloc(sizeof(*rc), GFP_KERNEL);
+		if (!rc)
+			goto out;
+		rc->rc_sequenceid = seqno;
+		rc->rc_slotid = slotid;
+		rcl->__nr_referring_calls++;
+		list_add(&rc->__list, &rcl->rcl_referring_calls);
+	}
+
+out:
+	if (!rcl->__nr_referring_calls) {
+		cb->cb_nr_referring_call_list--;
+		kfree(rcl);
+	}
+}
+
+/**
+ * nfsd41_cb_destroy_referring_call_list - release referring call info
+ * @cb: context of a callback that has completed
+ *
+ * Callers who allocate referring calls using nfsd41_cb_referring_call() must
+ * release those resources by calling nfsd41_cb_destroy_referring_call_list.
+ *
+ * Caller serializes access to @cb.
+ */
+void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb)
+{
+	struct nfsd4_referring_call_list *rcl;
+	struct nfsd4_referring_call *rc;
+
+	while (!list_empty(&cb->cb_referring_call_list)) {
+		rcl = list_first_entry(&cb->cb_referring_call_list,
+				       struct nfsd4_referring_call_list,
+				       __list);
+
+		while (!list_empty(&rcl->rcl_referring_calls)) {
+			rc = list_first_entry(&rcl->rcl_referring_calls,
+					      struct nfsd4_referring_call,
+					      __list);
+			list_del(&rc->__list);
+			kfree(rc);
+		}
+		list_del(&rcl->__list);
+		kfree(rcl);
+	}
+}
+
 static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfsd4_callback *cb = calldata;
@@ -1625,6 +1745,8 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
 	cb->cb_status = 0;
 	cb->cb_need_restart = false;
 	cb->cb_held_slot = -1;
+	cb->cb_nr_referring_call_list = 0;
+	INIT_LIST_HEAD(&cb->cb_referring_call_list);
 }
 
 /**
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 74d2d7b42676..b4af840fc4f9 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -64,6 +64,21 @@ typedef struct {
 	refcount_t		cs_count;
 } copy_stateid_t;
 
+struct nfsd4_referring_call {
+	struct list_head	__list;
+
+	u32			rc_sequenceid;
+	u32			rc_slotid;
+};
+
+struct nfsd4_referring_call_list {
+	struct list_head	__list;
+
+	struct nfs4_sessionid	rcl_sessionid;
+	int			__nr_referring_calls;
+	struct list_head	rcl_referring_calls;
+};
+
 struct nfsd4_callback {
 	struct nfs4_client *cb_clp;
 	struct rpc_message cb_msg;
@@ -73,6 +88,9 @@ struct nfsd4_callback {
 	int cb_status;
 	int cb_held_slot;
 	bool cb_need_restart;
+
+	int cb_nr_referring_call_list;
+	struct list_head cb_referring_call_list;
 };
 
 struct nfsd4_callback_ops {
@@ -777,6 +795,10 @@ extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
 extern void nfsd4_probe_callback(struct nfs4_client *clp);
 extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
 extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
+extern void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
+				     struct nfs4_sessionid *sessionid,
+				     u32 slotid, u32 seqno);
+extern void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb);
 extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
 		const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op);
 extern bool nfsd4_run_cb(struct nfsd4_callback *cb);
diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
index f1a315cd31b7..f4e29c0c701c 100644
--- a/fs/nfsd/xdr4cb.h
+++ b/fs/nfsd/xdr4cb.h
@@ -6,8 +6,11 @@
 #define cb_compound_enc_hdr_sz		4
 #define cb_compound_dec_hdr_sz		(3 + (NFS4_MAXTAGLEN >> 2))
 #define sessionid_sz			(NFS4_MAX_SESSIONID_LEN >> 2)
+#define enc_referring_call4_sz		(1 + 1)
+#define enc_referring_call_list4_sz	(sessionid_sz + 1 + \
+					enc_referring_call4_sz)
 #define cb_sequence_enc_sz		(sessionid_sz + 4 +             \
-					1 /* no referring calls list yet */)
+					enc_referring_call_list4_sz)
 #define cb_sequence_dec_sz		(op_dec_sz + sessionid_sz + 4)
 
 #define op_enc_sz			1
-- 
2.47.0

[PATCH v2 4/5] NFSD: Record each NFSv4 call's session slot index

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

The slot index number of the current COMPOUND has, until now, not
been needed outside of nfsd4_sequence(). But to record the tuple
that represents a referring call, the slot number will be needed
when processing subsequent operations in the COMPOUND.

Refactor the code that allocates a new struct nfsd4_slot to ensure
that the new sl_index field is always correctly initialized.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4state.c | 38 +++++++++++++++++++++-----------------
 fs/nfsd/state.h     |  1 +
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 153eeea2c7c9..d25f2a65c2bc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1989,26 +1989,30 @@ reduce_session_slots(struct nfsd4_session *ses, int dec)
 	return ret;
 }
 
-/*
- * We don't actually need to cache the rpc and session headers, so we
- * can allocate a little less for each slot:
- */
-static inline u32 slot_bytes(struct nfsd4_channel_attrs *ca)
+static struct nfsd4_slot *nfsd4_alloc_slot(struct nfsd4_channel_attrs *fattrs,
+					   int index, gfp_t gfp)
 {
-	u32 size;
+	struct nfsd4_slot *slot;
+	size_t size;
 
-	if (ca->maxresp_cached < NFSD_MIN_HDR_SEQ_SZ)
-		size = 0;
-	else
-		size = ca->maxresp_cached - NFSD_MIN_HDR_SEQ_SZ;
-	return size + sizeof(struct nfsd4_slot);
+	/*
+	 * The RPC and NFS session headers are never saved in
+	 * the slot reply cache buffer.
+	 */
+	size = fattrs->maxresp_cached < NFSD_MIN_HDR_SEQ_SZ ?
+		0 : fattrs->maxresp_cached - NFSD_MIN_HDR_SEQ_SZ;
+
+	slot = kzalloc(struct_size(slot, sl_data, size), gfp);
+	if (!slot)
+		return NULL;
+	slot->sl_index = index;
+	return slot;
 }
 
 static struct nfsd4_session *alloc_session(struct nfsd4_channel_attrs *fattrs,
 					   struct nfsd4_channel_attrs *battrs)
 {
 	int numslots = fattrs->maxreqs;
-	int slotsize = slot_bytes(fattrs);
 	struct nfsd4_session *new;
 	struct nfsd4_slot *slot;
 	int i;
@@ -2017,14 +2021,14 @@ static struct nfsd4_session *alloc_session(struct nfsd4_channel_attrs *fattrs,
 	if (!new)
 		return NULL;
 	xa_init(&new->se_slots);
-	/* allocate each struct nfsd4_slot and data cache in one piece */
-	slot = kzalloc(slotsize, GFP_KERNEL);
+
+	slot = nfsd4_alloc_slot(fattrs, 0, GFP_KERNEL);
 	if (!slot || xa_is_err(xa_store(&new->se_slots, 0, slot, GFP_KERNEL)))
 		goto out_free;
 
 	for (i = 1; i < numslots; i++) {
 		const gfp_t gfp = GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN;
-		slot = kzalloc(slotsize, gfp);
+		slot = nfsd4_alloc_slot(fattrs, i, gfp);
 		if (!slot)
 			break;
 		if (xa_is_err(xa_store(&new->se_slots, i, slot, gfp))) {
@@ -4438,8 +4442,8 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			 * spinlock, and only succeeds if there is
 			 * plenty of memory.
 			 */
-			slot = kzalloc(slot_bytes(&session->se_fchannel),
-				       GFP_NOWAIT);
+			slot = nfsd4_alloc_slot(&session->se_fchannel, s,
+						GFP_NOWAIT);
 			prev_slot = xa_load(&session->se_slots, s);
 			if (xa_is_value(prev_slot) && slot) {
 				slot->sl_seqid = xa_to_value(prev_slot);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index b4af840fc4f9..a971c8503c37 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -279,6 +279,7 @@ struct nfsd4_slot {
 	u32	sl_seqid;
 	__be32	sl_status;
 	struct svc_cred sl_cred;
+	u32	sl_index;
 	u32	sl_datalen;
 	u16	sl_opcnt;
 	u16	sl_generation;
-- 
2.47.0

[PATCH v2 5/5] NFSD: Use a referring call list for CB_OFFLOAD

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

Help the client resolve the race between the reply to an
asynchronous COPY reply and the associated CB_OFFLOAD callback by
planting the session, slot, and sequence number of the COPY in the
CB_SEQUENCE contained in the CB_OFFLOAD COMPOUND.

Suggested-by: Trond Myklebust <trondmy@hammerspace.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4proc.c | 9 +++++++++
 fs/nfsd/xdr4.h     | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3431b695882d..48c1e3600d75 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1716,6 +1716,7 @@ static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
 			return 0;
 		}
 	}
+	nfsd41_cb_destroy_referring_call_list(cb);
 	return 1;
 }
 
@@ -1848,6 +1849,9 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
 
 	nfsd4_init_cb(&cbo->co_cb, copy->cp_clp, &nfsd4_cb_offload_ops,
 		      NFSPROC4_CLNT_CB_OFFLOAD);
+	nfsd41_cb_referring_call(&cbo->co_cb, &cbo->co_referring_sessionid,
+				 cbo->co_referring_slotid,
+				 cbo->co_referring_seqno);
 	trace_nfsd_cb_offload(copy->cp_clp, &cbo->co_res.cb_stateid,
 			      &cbo->co_fh, copy->cp_count, copy->nfserr);
 	nfsd4_run_cb(&cbo->co_cb);
@@ -1964,6 +1968,11 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		memcpy(&result->cb_stateid, &copy->cp_stateid.cs_stid,
 			sizeof(result->cb_stateid));
 		dup_copy_fields(copy, async_copy);
+		memcpy(async_copy->cp_cb_offload.co_referring_sessionid.data,
+		       cstate->session->se_sessionid.data,
+		       NFS4_MAX_SESSIONID_LEN);
+		async_copy->cp_cb_offload.co_referring_slotid = cstate->slot->sl_index;
+		async_copy->cp_cb_offload.co_referring_seqno = cstate->slot->sl_seqid;
 		async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
 				async_copy, "%s", "copy thread");
 		if (IS_ERR(async_copy->copy_task))
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index c26ba86dbdfd..aa2a356da784 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -676,6 +676,10 @@ struct nfsd4_cb_offload {
 	__be32			co_nfserr;
 	unsigned int		co_retries;
 	struct knfsd_fh		co_fh;
+
+	struct nfs4_sessionid	co_referring_sessionid;
+	u32			co_referring_slotid;
+	u32			co_referring_seqno;
 };
 
 struct nfsd4_copy {
-- 
2.47.0

Re: [PATCH v2 1/5] NFSD: OFFLOAD_CANCEL should mark an async COPY as completed

[Jeff Layton]

On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Update the status of an async COPY operation when it has been
> stopped. OFFLOAD_STATUS needs to indicate that the COPY is no longer
> running.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4proc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f6e06c779d09..9a0e68aa246f 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1379,8 +1379,11 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
>  static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>  {
>  	trace_nfsd_copy_async_cancel(copy);
> -	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags))
> +	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags)) {
>  		kthread_stop(copy->copy_task);
> +		copy->nfserr = nfs_ok;
> +		set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
> +	}
>  	nfs4_put_copy(copy);
>  }
>  

STOPPED and COMPLETED seem to basically track each other. What's the
distinction between the two bits? When is one set, but not the other?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 1/5] NFSD: OFFLOAD_CANCEL should mark an async COPY as completed

[Chuck Lever]

On 3/2/25 4:35 PM, Jeff Layton wrote:
> On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>
>> Update the status of an async COPY operation when it has been
>> stopped. OFFLOAD_STATUS needs to indicate that the COPY is no longer
>> running.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  fs/nfsd/nfs4proc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index f6e06c779d09..9a0e68aa246f 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1379,8 +1379,11 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
>>  static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>>  {
>>  	trace_nfsd_copy_async_cancel(copy);
>> -	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags))
>> +	if (!test_and_set_bit(NFSD4_COPY_F_STOPPED, &copy->cp_flags)) {
>>  		kthread_stop(copy->copy_task);
>> +		copy->nfserr = nfs_ok;
>> +		set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
>> +	}
>>  	nfs4_put_copy(copy);
>>  }
>>  
> 
> STOPPED and COMPLETED seem to basically track each other. What's the
> distinction between the two bits? When is one set, but not the other?

IIRC:

COMPLETED means the background COPY is no longer running. It prevents
kthread_stop() from being called more than once on the same kthread.

STOPPED means that the background COPY is no longer running because
something stopped it before it could finish normally -- more or less
this is like being signaled.

We could name it "ABEND" or something else.

-- 
Chuck Lever

Re: [PATCH v2 2/5] NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY

[Jeff Layton]

On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Try not to prolong the wait for completion of a COPY or COPY_NOTIFY
> operation.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4proc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 9a0e68aa246f..3431b695882d 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1712,7 +1712,7 @@ static int nfsd4_cb_offload_done(struct nfsd4_callback *cb,
>  	switch (task->tk_status) {
>  	case -NFS4ERR_DELAY:
>  		if (cbo->co_retries--) {
> -			rpc_delay(task, 1 * HZ);
> +			rpc_delay(task, HZ / 5);
>  			return 0;
>  		}
>  	}

This is fine for now.

What I think we probably ought to do for all the callbacks is implement
a sliding delay window, and handle it in the common rpc_call_done
handling code. 
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 3/5] NFSD: Implement CB_SEQUENCE referring call lists

[Jeff Layton]

On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> We have yet to implement a mechanism in NFSD for resolving races
> between a server's reply and a related callback operation. For
> example, a CB_OFFLOAD callback can race with the matching COPY
> response. The client will not recognize the copy state ID in the
> CB_OFFLOAD callback until the COPY response arrives.
> 
> Trond adds:
> > It is also needed for the same kind of race with delegation
> > recalls, layout recalls, CB_NOTIFY_DEVICEID and would also be
> > helpful (although not as strongly required) for CB_NOTIFY_LOCK.
> 
> RFC 8881 Section 20.9.3 describes referring call lists this way:
> > The csa_referring_call_lists array is the list of COMPOUND
> > requests, identified by session ID, slot ID, and sequence ID.
> > These are requests that the client previously sent to the server.
> > These previous requests created state that some operation(s) in
> > the same CB_COMPOUND as the csa_referring_call_lists are
> > identifying. A session ID is included because leased state is tied
> > to a client ID, and a client ID can have multiple sessions. See
> > Section 2.10.6.3.
> 
> Introduce the XDR infrastructure for populating the
> csa_referring_call_lists argument of CB_SEQUENCE. Subsequent patches
> will put the referring call list to use.
> 
> Note that cb_sequence_enc_sz estimates that only zero or one rcl is
> included in each CB_SEQUENCE, but the new infrastructure can
> manage any number of referring calls.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
>  fs/nfsd/state.h        |  22 +++++++
>  fs/nfsd/xdr4cb.h       |   5 +-
>  3 files changed, 153 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 484077200c5d..f1fffff69330 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -419,6 +419,29 @@ static u32 highest_slotid(struct nfsd4_session *ses)
>  	return idx;
>  }
>  
> +static void
> +encode_referring_call4(struct xdr_stream *xdr,
> +		       const struct nfsd4_referring_call *rc)
> +{
> +	encode_uint32(xdr, rc->rc_sequenceid);
> +	encode_uint32(xdr, rc->rc_slotid);
> +}
> +
> +static void
> +encode_referring_call_list4(struct xdr_stream *xdr,
> +			    const struct nfsd4_referring_call_list *rcl)
> +{
> +	struct nfsd4_referring_call *rc;
> +	__be32 *p;
> +
> +	p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN);
> +	xdr_encode_opaque_fixed(p, rcl->rcl_sessionid.data,
> +					NFS4_MAX_SESSIONID_LEN);
> +	encode_uint32(xdr, rcl->__nr_referring_calls);
> +	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list)
> +		encode_referring_call4(xdr, rc);
> +}
> +
>  /*
>   * CB_SEQUENCE4args
>   *
> @@ -436,6 +459,7 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
>  				    struct nfs4_cb_compound_hdr *hdr)
>  {
>  	struct nfsd4_session *session = cb->cb_clp->cl_cb_session;
> +	struct nfsd4_referring_call_list *rcl;
>  	__be32 *p;
>  
>  	if (hdr->minorversion == 0)
> @@ -444,12 +468,16 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
>  	encode_nfs_cb_opnum4(xdr, OP_CB_SEQUENCE);
>  	encode_sessionid4(xdr, session);
>  
> -	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4 + 4);
> +	p = xdr_reserve_space(xdr, XDR_UNIT * 4);
>  	*p++ = cpu_to_be32(session->se_cb_seq_nr[cb->cb_held_slot]);	/* csa_sequenceid */
>  	*p++ = cpu_to_be32(cb->cb_held_slot);		/* csa_slotid */
>  	*p++ = cpu_to_be32(highest_slotid(session)); /* csa_highest_slotid */
>  	*p++ = xdr_zero;			/* csa_cachethis */
> -	xdr_encode_empty_array(p);		/* csa_referring_call_lists */
> +
> +	/* csa_referring_call_lists */
> +	encode_uint32(xdr, cb->cb_nr_referring_call_list);
> +	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list)
> +		encode_referring_call_list4(xdr, rcl);
>  
>  	hdr->nops++;
>  }
> @@ -1306,10 +1334,102 @@ static void nfsd41_destroy_cb(struct nfsd4_callback *cb)
>  	nfsd41_cb_inflight_end(clp);
>  }
>  
> -/*
> - * TODO: cb_sequence should support referring call lists, cachethis,
> - * and mark callback channel down on communication errors.
> +/**
> + * nfsd41_cb_referring_call - add a referring call to a callback operation
> + * @cb: context of callback to add the rc to
> + * @sessionid: referring call's session ID
> + * @slotid: referring call's session slot index
> + * @seqno: referring call's slot sequence number
> + *
> + * Caller serializes access to @cb.
> + *
> + * NB: If memory allocation fails, the referring call is not added.
>   */
> +void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
> +			      struct nfs4_sessionid *sessionid,
> +			      u32 slotid, u32 seqno)
> +{
> +	struct nfsd4_referring_call_list *rcl;
> +	struct nfsd4_referring_call *rc;
> +	bool found;
> +
> +	might_sleep();
> +
> +	found = false;
> +	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list) {
> +		if (!memcmp(rcl->rcl_sessionid.data, sessionid->data,
> +			   NFS4_MAX_SESSIONID_LEN)) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		rcl = kmalloc(sizeof(*rcl), GFP_KERNEL);
> +		if (!rcl)
> +			return;
> +		memcpy(rcl->rcl_sessionid.data, sessionid->data,
> +		       NFS4_MAX_SESSIONID_LEN);
> +		rcl->__nr_referring_calls = 0;
> +		INIT_LIST_HEAD(&rcl->rcl_referring_calls);
> +		list_add(&rcl->__list, &cb->cb_referring_call_list);
> +		cb->cb_nr_referring_call_list++;
> +	}
> +
> +	found = false;
> +	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list) {
> +		if (rc->rc_sequenceid == seqno && rc->rc_slotid == slotid) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		rc = kmalloc(sizeof(*rc), GFP_KERNEL);
> +		if (!rc)
> +			goto out;
> +		rc->rc_sequenceid = seqno;
> +		rc->rc_slotid = slotid;
> +		rcl->__nr_referring_calls++;
> +		list_add(&rc->__list, &rcl->rcl_referring_calls);
> +	}
> +
> +out:
> +	if (!rcl->__nr_referring_calls) {
> +		cb->cb_nr_referring_call_list--;
> +		kfree(rcl);
> +	}
> +}
> +
> +/**
> + * nfsd41_cb_destroy_referring_call_list - release referring call info
> + * @cb: context of a callback that has completed
> + *
> + * Callers who allocate referring calls using nfsd41_cb_referring_call() must
> + * release those resources by calling nfsd41_cb_destroy_referring_call_list.
> + *
> + * Caller serializes access to @cb.
> + */
> +void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb)
> +{
> +	struct nfsd4_referring_call_list *rcl;
> +	struct nfsd4_referring_call *rc;
> +
> +	while (!list_empty(&cb->cb_referring_call_list)) {
> +		rcl = list_first_entry(&cb->cb_referring_call_list,
> +				       struct nfsd4_referring_call_list,
> +				       __list);
> +
> +		while (!list_empty(&rcl->rcl_referring_calls)) {
> +			rc = list_first_entry(&rcl->rcl_referring_calls,
> +					      struct nfsd4_referring_call,
> +					      __list);
> +			list_del(&rc->__list);
> +			kfree(rc);
> +		}
> +		list_del(&rcl->__list);
> +		kfree(rcl);
> +	}
> +}
> +
>  static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
>  {
>  	struct nfsd4_callback *cb = calldata;
> @@ -1625,6 +1745,8 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
>  	cb->cb_status = 0;
>  	cb->cb_need_restart = false;
>  	cb->cb_held_slot = -1;
> +	cb->cb_nr_referring_call_list = 0;
> +	INIT_LIST_HEAD(&cb->cb_referring_call_list);
>  }
>  
>  /**
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 74d2d7b42676..b4af840fc4f9 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -64,6 +64,21 @@ typedef struct {
>  	refcount_t		cs_count;
>  } copy_stateid_t;
>  
> +struct nfsd4_referring_call {
> +	struct list_head	__list;
> +
> +	u32			rc_sequenceid;
> +	u32			rc_slotid;
> +};
> +
> +struct nfsd4_referring_call_list {
> +	struct list_head	__list;
> +
> +	struct nfs4_sessionid	rcl_sessionid;
> +	int			__nr_referring_calls;
> +	struct list_head	rcl_referring_calls;
> +};
> +

This set of nested lists is rather complex. Did you consider keeping a
single list and just adding the sessionid to nfsd4_referring_call? I
suppose that might mean you'd have to do more sessionid comparisons but
in general, I'd expect these lists to be short.


>  struct nfsd4_callback {
>  	struct nfs4_client *cb_clp;
>  	struct rpc_message cb_msg;
> @@ -73,6 +88,9 @@ struct nfsd4_callback {
>  	int cb_status;
>  	int cb_held_slot;
>  	bool cb_need_restart;
> +
> +	int cb_nr_referring_call_list;
> +	struct list_head cb_referring_call_list;
>  };
>  
>  struct nfsd4_callback_ops {
> @@ -777,6 +795,10 @@ extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
>  extern void nfsd4_probe_callback(struct nfs4_client *clp);
>  extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
>  extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
> +extern void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
> +				     struct nfs4_sessionid *sessionid,
> +				     u32 slotid, u32 seqno);
> +extern void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb);
>  extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
>  		const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op);
>  extern bool nfsd4_run_cb(struct nfsd4_callback *cb);
> diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> index f1a315cd31b7..f4e29c0c701c 100644
> --- a/fs/nfsd/xdr4cb.h
> +++ b/fs/nfsd/xdr4cb.h
> @@ -6,8 +6,11 @@
>  #define cb_compound_enc_hdr_sz		4
>  #define cb_compound_dec_hdr_sz		(3 + (NFS4_MAXTAGLEN >> 2))
>  #define sessionid_sz			(NFS4_MAX_SESSIONID_LEN >> 2)
> +#define enc_referring_call4_sz		(1 + 1)
> +#define enc_referring_call_list4_sz	(sessionid_sz + 1 + \
> +					enc_referring_call4_sz)
>  #define cb_sequence_enc_sz		(sessionid_sz + 4 +             \
> -					1 /* no referring calls list yet */)
> +					enc_referring_call_list4_sz)
>  #define cb_sequence_dec_sz		(op_dec_sz + sessionid_sz + 4)
>  
>  #define op_enc_sz			1

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2 3/5] NFSD: Implement CB_SEQUENCE referring call lists

[Chuck Lever]

On 3/3/25 1:05 PM, Jeff Layton wrote:
> On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>
>> We have yet to implement a mechanism in NFSD for resolving races
>> between a server's reply and a related callback operation. For
>> example, a CB_OFFLOAD callback can race with the matching COPY
>> response. The client will not recognize the copy state ID in the
>> CB_OFFLOAD callback until the COPY response arrives.
>>
>> Trond adds:
>>> It is also needed for the same kind of race with delegation
>>> recalls, layout recalls, CB_NOTIFY_DEVICEID and would also be
>>> helpful (although not as strongly required) for CB_NOTIFY_LOCK.
>>
>> RFC 8881 Section 20.9.3 describes referring call lists this way:
>>> The csa_referring_call_lists array is the list of COMPOUND
>>> requests, identified by session ID, slot ID, and sequence ID.
>>> These are requests that the client previously sent to the server.
>>> These previous requests created state that some operation(s) in
>>> the same CB_COMPOUND as the csa_referring_call_lists are
>>> identifying. A session ID is included because leased state is tied
>>> to a client ID, and a client ID can have multiple sessions. See
>>> Section 2.10.6.3.
>>
>> Introduce the XDR infrastructure for populating the
>> csa_referring_call_lists argument of CB_SEQUENCE. Subsequent patches
>> will put the referring call list to use.
>>
>> Note that cb_sequence_enc_sz estimates that only zero or one rcl is
>> included in each CB_SEQUENCE, but the new infrastructure can
>> manage any number of referring calls.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
>>  fs/nfsd/state.h        |  22 +++++++
>>  fs/nfsd/xdr4cb.h       |   5 +-
>>  3 files changed, 153 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>> index 484077200c5d..f1fffff69330 100644
>> --- a/fs/nfsd/nfs4callback.c
>> +++ b/fs/nfsd/nfs4callback.c
>> @@ -419,6 +419,29 @@ static u32 highest_slotid(struct nfsd4_session *ses)
>>  	return idx;
>>  }
>>  
>> +static void
>> +encode_referring_call4(struct xdr_stream *xdr,
>> +		       const struct nfsd4_referring_call *rc)
>> +{
>> +	encode_uint32(xdr, rc->rc_sequenceid);
>> +	encode_uint32(xdr, rc->rc_slotid);
>> +}
>> +
>> +static void
>> +encode_referring_call_list4(struct xdr_stream *xdr,
>> +			    const struct nfsd4_referring_call_list *rcl)
>> +{
>> +	struct nfsd4_referring_call *rc;
>> +	__be32 *p;
>> +
>> +	p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN);
>> +	xdr_encode_opaque_fixed(p, rcl->rcl_sessionid.data,
>> +					NFS4_MAX_SESSIONID_LEN);
>> +	encode_uint32(xdr, rcl->__nr_referring_calls);
>> +	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list)
>> +		encode_referring_call4(xdr, rc);
>> +}
>> +
>>  /*
>>   * CB_SEQUENCE4args
>>   *
>> @@ -436,6 +459,7 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
>>  				    struct nfs4_cb_compound_hdr *hdr)
>>  {
>>  	struct nfsd4_session *session = cb->cb_clp->cl_cb_session;
>> +	struct nfsd4_referring_call_list *rcl;
>>  	__be32 *p;
>>  
>>  	if (hdr->minorversion == 0)
>> @@ -444,12 +468,16 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,
>>  	encode_nfs_cb_opnum4(xdr, OP_CB_SEQUENCE);
>>  	encode_sessionid4(xdr, session);
>>  
>> -	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4 + 4);
>> +	p = xdr_reserve_space(xdr, XDR_UNIT * 4);
>>  	*p++ = cpu_to_be32(session->se_cb_seq_nr[cb->cb_held_slot]);	/* csa_sequenceid */
>>  	*p++ = cpu_to_be32(cb->cb_held_slot);		/* csa_slotid */
>>  	*p++ = cpu_to_be32(highest_slotid(session)); /* csa_highest_slotid */
>>  	*p++ = xdr_zero;			/* csa_cachethis */
>> -	xdr_encode_empty_array(p);		/* csa_referring_call_lists */
>> +
>> +	/* csa_referring_call_lists */
>> +	encode_uint32(xdr, cb->cb_nr_referring_call_list);
>> +	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list)
>> +		encode_referring_call_list4(xdr, rcl);
>>  
>>  	hdr->nops++;
>>  }
>> @@ -1306,10 +1334,102 @@ static void nfsd41_destroy_cb(struct nfsd4_callback *cb)
>>  	nfsd41_cb_inflight_end(clp);
>>  }
>>  
>> -/*
>> - * TODO: cb_sequence should support referring call lists, cachethis,
>> - * and mark callback channel down on communication errors.
>> +/**
>> + * nfsd41_cb_referring_call - add a referring call to a callback operation
>> + * @cb: context of callback to add the rc to
>> + * @sessionid: referring call's session ID
>> + * @slotid: referring call's session slot index
>> + * @seqno: referring call's slot sequence number
>> + *
>> + * Caller serializes access to @cb.
>> + *
>> + * NB: If memory allocation fails, the referring call is not added.
>>   */
>> +void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
>> +			      struct nfs4_sessionid *sessionid,
>> +			      u32 slotid, u32 seqno)
>> +{
>> +	struct nfsd4_referring_call_list *rcl;
>> +	struct nfsd4_referring_call *rc;
>> +	bool found;
>> +
>> +	might_sleep();
>> +
>> +	found = false;
>> +	list_for_each_entry(rcl, &cb->cb_referring_call_list, __list) {
>> +		if (!memcmp(rcl->rcl_sessionid.data, sessionid->data,
>> +			   NFS4_MAX_SESSIONID_LEN)) {
>> +			found = true;
>> +			break;
>> +		}
>> +	}
>> +	if (!found) {
>> +		rcl = kmalloc(sizeof(*rcl), GFP_KERNEL);
>> +		if (!rcl)
>> +			return;
>> +		memcpy(rcl->rcl_sessionid.data, sessionid->data,
>> +		       NFS4_MAX_SESSIONID_LEN);
>> +		rcl->__nr_referring_calls = 0;
>> +		INIT_LIST_HEAD(&rcl->rcl_referring_calls);
>> +		list_add(&rcl->__list, &cb->cb_referring_call_list);
>> +		cb->cb_nr_referring_call_list++;
>> +	}
>> +
>> +	found = false;
>> +	list_for_each_entry(rc, &rcl->rcl_referring_calls, __list) {
>> +		if (rc->rc_sequenceid == seqno && rc->rc_slotid == slotid) {
>> +			found = true;
>> +			break;
>> +		}
>> +	}
>> +	if (!found) {
>> +		rc = kmalloc(sizeof(*rc), GFP_KERNEL);
>> +		if (!rc)
>> +			goto out;
>> +		rc->rc_sequenceid = seqno;
>> +		rc->rc_slotid = slotid;
>> +		rcl->__nr_referring_calls++;
>> +		list_add(&rc->__list, &rcl->rcl_referring_calls);
>> +	}
>> +
>> +out:
>> +	if (!rcl->__nr_referring_calls) {
>> +		cb->cb_nr_referring_call_list--;
>> +		kfree(rcl);
>> +	}
>> +}
>> +
>> +/**
>> + * nfsd41_cb_destroy_referring_call_list - release referring call info
>> + * @cb: context of a callback that has completed
>> + *
>> + * Callers who allocate referring calls using nfsd41_cb_referring_call() must
>> + * release those resources by calling nfsd41_cb_destroy_referring_call_list.
>> + *
>> + * Caller serializes access to @cb.
>> + */
>> +void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb)
>> +{
>> +	struct nfsd4_referring_call_list *rcl;
>> +	struct nfsd4_referring_call *rc;
>> +
>> +	while (!list_empty(&cb->cb_referring_call_list)) {
>> +		rcl = list_first_entry(&cb->cb_referring_call_list,
>> +				       struct nfsd4_referring_call_list,
>> +				       __list);
>> +
>> +		while (!list_empty(&rcl->rcl_referring_calls)) {
>> +			rc = list_first_entry(&rcl->rcl_referring_calls,
>> +					      struct nfsd4_referring_call,
>> +					      __list);
>> +			list_del(&rc->__list);
>> +			kfree(rc);
>> +		}
>> +		list_del(&rcl->__list);
>> +		kfree(rcl);
>> +	}
>> +}
>> +
>>  static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
>>  {
>>  	struct nfsd4_callback *cb = calldata;
>> @@ -1625,6 +1745,8 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
>>  	cb->cb_status = 0;
>>  	cb->cb_need_restart = false;
>>  	cb->cb_held_slot = -1;
>> +	cb->cb_nr_referring_call_list = 0;
>> +	INIT_LIST_HEAD(&cb->cb_referring_call_list);
>>  }
>>  
>>  /**
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index 74d2d7b42676..b4af840fc4f9 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -64,6 +64,21 @@ typedef struct {
>>  	refcount_t		cs_count;
>>  } copy_stateid_t;
>>  
>> +struct nfsd4_referring_call {
>> +	struct list_head	__list;
>> +
>> +	u32			rc_sequenceid;
>> +	u32			rc_slotid;
>> +};
>> +
>> +struct nfsd4_referring_call_list {
>> +	struct list_head	__list;
>> +
>> +	struct nfs4_sessionid	rcl_sessionid;
>> +	int			__nr_referring_calls;
>> +	struct list_head	rcl_referring_calls;
>> +};
>> +
> 
> This set of nested lists is rather complex. Did you consider keeping a
> single list and just adding the sessionid to nfsd4_referring_call? I
> suppose that might mean you'd have to do more sessionid comparisons but
> in general, I'd expect these lists to be short.

I agree that it's just a little bit more than we clearly need right now.

My eye is on replacing this code eventually with something generated by
xdrgen. xdrgen will manufacture something like this rather than
optimizing for only cases we can see right now in our rear-view mirror.

Even so, spec requires servers to implement client trunking, meaning
they have to handle multiple sessions per client ID. NFSD needs to be
prepared to handle the case where a client ID has several sessions
associated with it, only one of which has a backchannel.


>>  struct nfsd4_callback {
>>  	struct nfs4_client *cb_clp;
>>  	struct rpc_message cb_msg;
>> @@ -73,6 +88,9 @@ struct nfsd4_callback {
>>  	int cb_status;
>>  	int cb_held_slot;
>>  	bool cb_need_restart;
>> +
>> +	int cb_nr_referring_call_list;
>> +	struct list_head cb_referring_call_list;
>>  };
>>  
>>  struct nfsd4_callback_ops {
>> @@ -777,6 +795,10 @@ extern __be32 nfs4_check_open_reclaim(struct nfs4_client *);
>>  extern void nfsd4_probe_callback(struct nfs4_client *clp);
>>  extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
>>  extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
>> +extern void nfsd41_cb_referring_call(struct nfsd4_callback *cb,
>> +				     struct nfs4_sessionid *sessionid,
>> +				     u32 slotid, u32 seqno);
>> +extern void nfsd41_cb_destroy_referring_call_list(struct nfsd4_callback *cb);
>>  extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
>>  		const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op);
>>  extern bool nfsd4_run_cb(struct nfsd4_callback *cb);
>> diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
>> index f1a315cd31b7..f4e29c0c701c 100644
>> --- a/fs/nfsd/xdr4cb.h
>> +++ b/fs/nfsd/xdr4cb.h
>> @@ -6,8 +6,11 @@
>>  #define cb_compound_enc_hdr_sz		4
>>  #define cb_compound_dec_hdr_sz		(3 + (NFS4_MAXTAGLEN >> 2))
>>  #define sessionid_sz			(NFS4_MAX_SESSIONID_LEN >> 2)
>> +#define enc_referring_call4_sz		(1 + 1)
>> +#define enc_referring_call_list4_sz	(sessionid_sz + 1 + \
>> +					enc_referring_call4_sz)
>>  #define cb_sequence_enc_sz		(sessionid_sz + 4 +             \
>> -					1 /* no referring calls list yet */)
>> +					enc_referring_call_list4_sz)
>>  #define cb_sequence_dec_sz		(op_dec_sz + sessionid_sz + 4)
>>  
>>  #define op_enc_sz			1
> 


-- 
Chuck Lever

Re: [PATCH v2 0/5] Implement referring call lists for CB_OFFLOAD

[Jeff Layton]

On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> I've built a naive proof-of-concept of the csa_referring_call_list
> argument of the CB_SEQUENCE operation, and hooked it up for the
> CB_OFFLOAD callback operation.
> 
> This has been pushed to my kernel.org "fix-async-copy" branch for
> folks to play around with.
> 
> I've done some basic testing with a server that ensures the
> CB_OFFLOAD callback is sent before the COPY reply, while running a
> network capture. Operation appears correct, Wireshark is happy
> with the construction of the XDR, and the CB_SEQUENCE arguments
> match the SEQUENCE operation in the COPY COMPOUND.
> 
> I'd like to include this series in nfsd-testing.
> 
> Changes since RFC:
> - Add a field to struct nfsd4_slot that records its table index
> - Include a few additional COPY-related fixes
> - Some operational testing has been done
> 
> Chuck Lever (5):
>   NFSD: OFFLOAD_CANCEL should mark an async COPY as completed
>   NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY
>   NFSD: Implement CB_SEQUENCE referring call lists
>   NFSD: Record each NFSv4 call's session slot index
>   NFSD: Use a referring call list for CB_OFFLOAD
> 
>  fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfs4proc.c     |  16 ++++-
>  fs/nfsd/nfs4state.c    |  38 ++++++------
>  fs/nfsd/state.h        |  23 +++++++
>  fs/nfsd/xdr4.h         |   4 ++
>  fs/nfsd/xdr4cb.h       |   5 +-
>  6 files changed, 193 insertions(+), 25 deletions(-)
> 

I think this all looks good for a first pass, and should be OK for
COPY. You can add:


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


I think we'll eventually want this for longer-lived stateids too.
Specifically:

    CB_RECALL
    CB_LAYOUTRECALL
    CB_NOTIFY_LOCK

The main thing missing for that is the ability to free referring call
records once we ensure that the client has seen the reply. For
instance, if nfsd records a referring call on slot:seq 1:2, then once
it sees a SEQUENCE for 1:3, then it doesn't need to keep around the
referring call for 1:2. The server knows that call is no longer in
flight so it's no longer needed.

If we don't do that, then we could end up with rather long referring
call lists, with a bunch of long-completed calls.

Re: [PATCH v2 0/5] Implement referring call lists for CB_OFFLOAD

[Chuck Lever]

On 3/7/25 10:00 AM, Jeff Layton wrote:
> On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>
>> I've built a naive proof-of-concept of the csa_referring_call_list
>> argument of the CB_SEQUENCE operation, and hooked it up for the
>> CB_OFFLOAD callback operation.
>>
>> This has been pushed to my kernel.org "fix-async-copy" branch for
>> folks to play around with.
>>
>> I've done some basic testing with a server that ensures the
>> CB_OFFLOAD callback is sent before the COPY reply, while running a
>> network capture. Operation appears correct, Wireshark is happy
>> with the construction of the XDR, and the CB_SEQUENCE arguments
>> match the SEQUENCE operation in the COPY COMPOUND.
>>
>> I'd like to include this series in nfsd-testing.
>>
>> Changes since RFC:
>> - Add a field to struct nfsd4_slot that records its table index
>> - Include a few additional COPY-related fixes
>> - Some operational testing has been done
>>
>> Chuck Lever (5):
>>   NFSD: OFFLOAD_CANCEL should mark an async COPY as completed
>>   NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY
>>   NFSD: Implement CB_SEQUENCE referring call lists
>>   NFSD: Record each NFSv4 call's session slot index
>>   NFSD: Use a referring call list for CB_OFFLOAD
>>
>>  fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
>>  fs/nfsd/nfs4proc.c     |  16 ++++-
>>  fs/nfsd/nfs4state.c    |  38 ++++++------
>>  fs/nfsd/state.h        |  23 +++++++
>>  fs/nfsd/xdr4.h         |   4 ++
>>  fs/nfsd/xdr4cb.h       |   5 +-
>>  6 files changed, 193 insertions(+), 25 deletions(-)
>>
> 
> I think this all looks good for a first pass, and should be OK for
> COPY. You can add:
> 
> 
>     Reviewed-by: Jeff Layton <jlayton@kernel.org>

Thanks!


> I think we'll eventually want this for longer-lived stateids too.
> Specifically:
> 
>     CB_RECALL
>     CB_LAYOUTRECALL
>     CB_NOTIFY_LOCK
> 
> The main thing missing for that is the ability to free referring call
> records once we ensure that the client has seen the reply. For
> instance, if nfsd records a referring call on slot:seq 1:2, then once
> it sees a SEQUENCE for 1:3, then it doesn't need to keep around the
> referring call for 1:2. The server knows that call is no longer in
> flight so it's no longer needed.
> 
> If we don't do that, then we could end up with rather long referring
> call lists, with a bunch of long-completed calls.

Agreed that RCLs will have uses outside of COPY. I don't have a good
understanding of the use cases you mention above, so it would delay
RCL support in CB_OFFLOAD quite a bit if we were to wait for the
implementation of the other use cases.


-- 
Chuck Lever

Re: [PATCH v2 0/5] Implement referring call lists for CB_OFFLOAD

[Jeff Layton]

On Fri, 2025-03-07 at 11:00 -0500, Chuck Lever wrote:
> On 3/7/25 10:00 AM, Jeff Layton wrote:
> > On Sat, 2025-03-01 at 13:31 -0500, cel@kernel.org wrote:
> > > From: Chuck Lever <chuck.lever@oracle.com>
> > > 
> > > I've built a naive proof-of-concept of the csa_referring_call_list
> > > argument of the CB_SEQUENCE operation, and hooked it up for the
> > > CB_OFFLOAD callback operation.
> > > 
> > > This has been pushed to my kernel.org "fix-async-copy" branch for
> > > folks to play around with.
> > > 
> > > I've done some basic testing with a server that ensures the
> > > CB_OFFLOAD callback is sent before the COPY reply, while running a
> > > network capture. Operation appears correct, Wireshark is happy
> > > with the construction of the XDR, and the CB_SEQUENCE arguments
> > > match the SEQUENCE operation in the COPY COMPOUND.
> > > 
> > > I'd like to include this series in nfsd-testing.
> > > 
> > > Changes since RFC:
> > > - Add a field to struct nfsd4_slot that records its table index
> > > - Include a few additional COPY-related fixes
> > > - Some operational testing has been done
> > > 
> > > Chuck Lever (5):
> > >   NFSD: OFFLOAD_CANCEL should mark an async COPY as completed
> > >   NFSD: Shorten CB_OFFLOAD response to NFS4ERR_DELAY
> > >   NFSD: Implement CB_SEQUENCE referring call lists
> > >   NFSD: Record each NFSv4 call's session slot index
> > >   NFSD: Use a referring call list for CB_OFFLOAD
> > > 
> > >  fs/nfsd/nfs4callback.c | 132 +++++++++++++++++++++++++++++++++++++++--
> > >  fs/nfsd/nfs4proc.c     |  16 ++++-
> > >  fs/nfsd/nfs4state.c    |  38 ++++++------
> > >  fs/nfsd/state.h        |  23 +++++++
> > >  fs/nfsd/xdr4.h         |   4 ++
> > >  fs/nfsd/xdr4cb.h       |   5 +-
> > >  6 files changed, 193 insertions(+), 25 deletions(-)
> > > 
> > 
> > I think this all looks good for a first pass, and should be OK for
> > COPY. You can add:
> > 
> > 
> >     Reviewed-by: Jeff Layton <jlayton@kernel.org>
> 
> Thanks!
> 
> 
> > I think we'll eventually want this for longer-lived stateids too.
> > Specifically:
> > 
> >     CB_RECALL
> >     CB_LAYOUTRECALL
> >     CB_NOTIFY_LOCK
> > 
> > The main thing missing for that is the ability to free referring call
> > records once we ensure that the client has seen the reply. For
> > instance, if nfsd records a referring call on slot:seq 1:2, then once
> > it sees a SEQUENCE for 1:3, then it doesn't need to keep around the
> > referring call for 1:2. The server knows that call is no longer in
> > flight so it's no longer needed.
> > 
> > If we don't do that, then we could end up with rather long referring
> > call lists, with a bunch of long-completed calls.
> 
> Agreed that RCLs will have uses outside of COPY. I don't have a good
> understanding of the use cases you mention above, so it would delay
> RCL support in CB_OFFLOAD quite a bit if we were to wait for the
> implementation of the other use cases.
> 
> 

The use-cases for CB_RECALL and CB_LAYOUTRECALL are basically the
same: 

Server handed out a delegation or layout stateid, and then had to
immediately recall it. We need the RCL in case the callback races ahead
of the reply that has the stateid.

Now that I look, I don't think it's actually needed for CB_NOTIFY_LOCK
(no stateid in the call), so we can strike that one off the list.
-- 
Jeff Layton <jlayton@kernel.org>