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

[Alexey Kuznetsov <kuznet@virtuozzo.com>]

Ack.

IMHO it can be committed, Kui Liu will check it later.

On Thu, Sep 10, 2026 at 11:26 PM Aleksandr Kharten
<aleksandr.kharten@virtuozzo.com> wrote:
>
> 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