[Devel] [PATCH vz10] fs/fuse kio: fix lost rpc response over rdma io

[Aleksandr Kharten <aleksandr.kharten@virtuozzo.com>]

Call ->done() when the peer's RDMA_READ_ACK arrives rather than waiting
for our own TX completion as well, so that a large message is reported
as sent before its response can come back. The tx itself is still
released only once both events are in, as the HCA may still be using
its MR.

https://virtuozzo.atlassian.net/browse/VSTOR-141673

---
A message larger than RDMA_THRESHOLD is not pushed to the peer but
advertised: rio_submit() registers its payload in an MR and sends a
header carrying the rkey, then the peer pulls the payload with RDMA
READ and reports RDMA_READ_ACK. Hence ->done() cannot be called right
after rio_tx_post_send() as it is for small messages, and
rio_handle_tx() used to call it only once both the peer's ack and our
own TX completion had arrived, counting the two events with
++tx->tx_state.

These two events are not ordered. The peer's HCA coalesces its RC
acknowledgment, so our TX completion can be reported hundreds of
microseconds after the peer has acked the RDMA READ and has already
answered the request. The response then reaches handle_response()
before pcs_rpc_sent() has put the request on ep->pending_queue, so
pcs_rpc_lookup_xid() finds nothing, the response is dropped silently,
and the request can only be completed by the kill calendar one
response timeout later.

Call ->done() when RDMA_READ_ACK arrives instead. The ack means the
peer has the payload, and it cannot be overtaken by the response, as
both come from the same RX queue and are handled in order, so the
request always reaches ep->pending_queue before its response is
looked up. Releasing the tx still requires both events, as the HCA
may still be using its MR; the payload it works on does not belong to
the message, which was either copied into the MR of the tx or is
pinned by the page references the tx took itself in
pcs_sgl_buf_init_from_msg().

Signed-off-by: Aleksandr Kharten <aleksandr.kharten@virtuozzo.com>
---
 fs/fuse/kio/pcs/pcs_rdma_io.c | 55 ++++++++++++++++++++++++++++++++-----------
 fs/fuse/kio/pcs/pcs_rdma_rw.h |  8 +++++++
 2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/fs/fuse/kio/pcs/pcs_rdma_io.c b/fs/fuse/kio/pcs/pcs_rdma_io.c
index 1c464e0e60f3..171e0bf3098b 100644
--- a/fs/fuse/kio/pcs/pcs_rdma_io.c
+++ b/fs/fuse/kio/pcs/pcs_rdma_io.c
@@ -60,8 +60,9 @@ struct rio_rx {
 
 enum {
 	TX_FREE,                   /* free tx request available for use */
-	TX_WAIT_FOR_TX_COMPL,      /* tx request sent, wait for TX completion */
-	TX_WAIT_FOR_READ_ACK,      /* wait for peer to ack RDMA read */
+	TX_WAIT_FOR_TX_COMPL,      /* wait for TX completion and RDMA read ack */
+	TX_WAIT_FOR_READ_ACK,      /* TX completed, wait for RDMA read ack */
+	TX_READ_ACKED,             /* msg completed on ack, wait for TX completion */
 	TX_MSG_DONE,               /* default: call msg->done() */
 	TX_SUBMIT_RDMA_READ_ACK,   /* let our peer know that our RDMA_READ is done */
 };
@@ -720,10 +721,9 @@ static int rio_submit(struct pcs_rdmaio *rio, struct pcs_msg *msg, int type, u64
 		 * lose an error.
 		 *
 		 * But, if the message is big enough to trigger RDMA READ
-		 * transfer, we are going to call ->done() callback after we
-		 * receive RDMA_READ_ACK message from our peer. Since messages
-		 * in a single RX queue are guaranteed to come in order, there
-		 * is no race in this case.
+		 * transfer, ->done() cannot be called here: the peer has not
+		 * pulled the payload yet. It is called from rio_handle_tx()
+		 * when RDMA_READ_ACK arrives, see the comment there.
 		 */
 		rio_msg_sent(rio, tx, msg, rb == NULL);
 	}
@@ -795,7 +795,21 @@ static inline void rio_kick_write_queue(struct pcs_rdmaio *rio)
 	rio_enable_kick(rio);
 }
 
-static void rio_handle_tx(struct pcs_rdmaio *rio, struct rio_tx *tx, int ok)
+/* Events a tx waits for, see rio_handle_tx() */
+enum rio_tx_event {
+	TX_EV_SEND_CQE,            /* TX completion from our HCA */
+	TX_EV_READ_ACK,            /* RDMA_READ_ACK from peer */
+};
+
+/*
+ * Handle one of the two events @tx waits for. They are not ordered: the peer
+ * may ack the RDMA read, and send its response, before our HCA reports the
+ * send. Hence msg->done() is called on TX_EV_READ_ACK, which the response
+ * cannot overtake as both come from the same RX queue, while the tx is
+ * released only after both events, as the HCA may still be using its MR.
+ */
+static void rio_handle_tx(struct pcs_rdmaio *rio, struct rio_tx *tx, int ok,
+			  enum rio_tx_event ev)
 {
 	struct pcs_msg *msg = tx->msg;
 	u64 xid = tx->xid;
@@ -814,9 +828,25 @@ static void rio_handle_tx(struct pcs_rdmaio *rio, struct rio_tx *tx, int ok)
 			rio->n_rdma_read_ongoing--;
 			break;
 		case TX_WAIT_FOR_TX_COMPL:
-		case TX_WAIT_FOR_READ_ACK:
-			if (++tx->tx_state != TX_MSG_DONE)
+			if (ev == TX_EV_SEND_CQE) {
+				/* Still waiting for the peer to ack RDMA read */
+				tx->tx_state = TX_WAIT_FOR_READ_ACK;
 				return;
+			}
+			/*
+			 * The peer is done with the payload, complete msg now
+			 * and keep the tx until our own TX completion arrives.
+			 * The tx must keep no reference to msg, which may be
+			 * freed by the completion below.
+			 */
+			tx->tx_state = TX_READ_ACKED;
+			tx->msg = NULL;
+			pcs_rdma_msg_detach(&tx->send.msg);
+			break;
+		case TX_WAIT_FOR_READ_ACK:
+			rio_put_tx(rio->dev, tx);
+			break;
+		case TX_READ_ACKED:
 			rio_put_tx(rio->dev, tx);
 			break;
 		case TX_MSG_DONE:
@@ -932,7 +962,7 @@ static int rio_handle_rx_read_ack(struct pcs_rdmaio *rio,
 	list_for_each_entry(tx, &rio->active_txs, list)
 		if (tx->xid == rack->xid) {
 			list_del(&tx->list);
-			rio_handle_tx(rio, tx, !rack->status);
+			rio_handle_tx(rio, tx, !rack->status, TX_EV_READ_ACK);
 			return 0;
 		}
 
@@ -1073,7 +1103,7 @@ static void rio_tx_done(struct rio_cqe *cqe, bool sync_mode)
 	if (sync_mode)
 		rio_put_tx(rio->dev, tx);
 	else
-		rio_handle_tx(rio, tx, cqe->status == IB_WC_SUCCESS);
+		rio_handle_tx(rio, tx, cqe->status == IB_WC_SUCCESS, TX_EV_SEND_CQE);
 }
 
 static inline struct rio_cqe* rio_poll_cq(struct pcs_rdmaio *rio)
@@ -1258,9 +1288,6 @@ struct pcs_rdmaio* pcs_rdma_create(int hdr_size, struct rdma_cm_id *cmid,
 
 	BUG_ON(!mutex_is_locked(&ep->mutex));
 
-	BUILD_BUG_ON((TX_WAIT_FOR_READ_ACK - TX_WAIT_FOR_TX_COMPL) != 1);
-	BUILD_BUG_ON((TX_MSG_DONE - TX_WAIT_FOR_READ_ACK) != 1);
-
 	if (queue_depth < RIO_QUEUE_DEPTH)
 		queue_depth = RIO_QUEUE_DEPTH;
 	else if (queue_depth > RIO_MAX_QUEUE_DEPTH)
diff --git a/fs/fuse/kio/pcs/pcs_rdma_rw.h b/fs/fuse/kio/pcs/pcs_rdma_rw.h
index c8529ed16f13..cc7a3fef4863 100644
--- a/fs/fuse/kio/pcs/pcs_rdma_rw.h
+++ b/fs/fuse/kio/pcs/pcs_rdma_rw.h
@@ -188,4 +188,12 @@ int pcs_rdma_msg_init(struct pcs_rdma_msg *rdma_msg, struct pcs_msg *msg,
 		      bool try_to_map);
 void pcs_rdma_msg_destroy(struct pcs_rdma_msg *rdma_msg);
 
+/*
+ * Drop the reference to the msg being sent.
+ */
+static inline void pcs_rdma_msg_detach(struct pcs_rdma_msg *rdma_msg)
+{
+	rdma_msg->msg = NULL;
+}
+
 #endif /* _PCS_RMDA_RW_H_ */

---
base-commit: 3466ec0b2cdf9d49dbb36d7b5ed9b36dfb2fb04d
change-id: 20260909-b4-vstor-141673-fix-lost-rdma-reply-7ad74eb161d3

Best regards,
--  
Aleksandr Kharten <aleksandr.kharten@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel