Re: [RFC PATCH net-next v4 10/11] net/smc: adapt cursor update when sndbuf and peer DMB are merged

Jan Karcher
2024-03-21T08:12:37Z

On 17/03/2024 11:05, Wen Gu wrote:
> Since ghost sndbuf shares the same physical memory with peer DMB,
> the cursor update processing needs to be adapted to ensure that the
> data to be consumed won't be overwritten.
>
> So in this case, the fin_curs and sndbuf_space that were originally
> updated after sending the CDC message should be modified to not be
> update until the peer updates cons_curs.
>
> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
> ---
> net/smc/smc_cdc.c | 52 +++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
> index 3c06625ceb20..bf5b214ec15a 100644
> --- a/net/smc/smc_cdc.c
> +++ b/net/smc/smc_cdc.c
> @@ -18,6 +18,7 @@
> #include "smc_tx.h"
> #include "smc_rx.h"
> #include "smc_close.h"
> +#include "smc_ism.h"
>
> /********************************** send *************************************/
>
> @@ -255,17 +256,25 @@ int smcd_cdc_msg_send(struct smc_connection *conn)
> return rc;
> smc_curs_copy(&conn->rx_curs_confirmed, &curs, conn);
> conn->local_rx_ctrl.prod_flags.cons_curs_upd_req = 0;
> - /* Calculate transmitted data and increment free send buffer space */
> - diff = smc_curs_diff(conn->sndbuf_desc->len, &conn->tx_curs_fin,
> - &conn->tx_curs_sent);
> - /* increased by confirmed number of bytes */
> - smp_mb__before_atomic();
> - atomic_add(diff, &conn->sndbuf_space);
> - /* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
> - smp_mb__after_atomic();
> - smc_curs_copy(&conn->tx_curs_fin, &conn->tx_curs_sent, conn);
> + if (!smc_ism_support_dmb_nocopy(conn->lgr->smcd)) {
> + /* Ghost sndbuf shares the same memory region with
> + * peer DMB, so don't update the tx_curs_fin and
> + * sndbuf_space until peer has consumed the data.
> + */

Hi Wen Gu,

move this comment above the if please. Two consecutive multiline
comments are difficult to read here.

> + /* Calculate transmitted data and increment free
> + * send buffer space
> + */
> + diff = smc_curs_diff(conn->sndbuf_desc->len, &conn->tx_curs_fin,
> + &conn->tx_curs_sent);
> + /* increased by confirmed number of bytes */
> + smp_mb__before_atomic();
> + atomic_add(diff, &conn->sndbuf_space);
> + /* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
> + smp_mb__after_atomic();
> + smc_curs_copy(&conn->tx_curs_fin, &conn->tx_curs_sent, conn);
>
> - smc_tx_sndbuf_nonfull(smc);
> + smc_tx_sndbuf_nonfull(smc);
> + }
> return rc;
> }
>
> @@ -323,7 +332,7 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
> {
> union smc_host_cursor cons_old, prod_old;
> struct smc_connection *conn = &smc->conn;
> - int diff_cons, diff_prod;
> + int diff_cons, diff_prod, diff_tx;
>
> smc_curs_copy(&prod_old, &conn->local_rx_ctrl.prod, conn);
> smc_curs_copy(&cons_old, &conn->local_rx_ctrl.cons, conn);
> @@ -339,6 +348,27 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
> atomic_add(diff_cons, &conn->peer_rmbe_space);
> /* guarantee 0 <= peer_rmbe_space <= peer_rmbe_size */
> smp_mb__after_atomic();
> +
> + if (conn->lgr->is_smcd &&
> + smc_ism_support_dmb_nocopy(conn->lgr->smcd)) {
> + /* Ghost sndbuf shares the same memory region with
> + * peer RMB, so update tx_curs_fin and sndbuf_space
> + * when peer has consumed the data.
> + */

Same as above.

Thanks
- Jan

> + /* calculate peer rmb consumed data */
> + diff_tx = smc_curs_diff(conn->sndbuf_desc->len,
> + &conn->tx_curs_fin,
> + &conn->local_rx_ctrl.cons);
> + /* increase local sndbuf space and fin_curs */
> + smp_mb__before_atomic();
> + atomic_add(diff_tx, &conn->sndbuf_space);
> + /* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
> + smp_mb__after_atomic();
> + smc_curs_copy(&conn->tx_curs_fin,
> + &conn->local_rx_ctrl.cons, conn);
> +
> + smc_tx_sndbuf_nonfull(smc);
> + }
> }
>
> diff_prod = smc_curs_diff(conn->rmb_desc->len, &prod_old,