[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <72c046f3-1eb5-44f8-b32b-1ff6471943a2@linux.alibaba.com>
Date: Fri, 22 Mar 2024 09:49:00 +0800
From: Wen Gu <guwen@...ux.alibaba.com>
To: Jan Karcher <jaka@...ux.ibm.com>, wintera@...ux.ibm.com,
twinkler@...ux.ibm.com, hca@...ux.ibm.com, gor@...ux.ibm.com,
agordeev@...ux.ibm.com, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, wenjia@...ux.ibm.com
Cc: borntraeger@...ux.ibm.com, svens@...ux.ibm.com,
alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next v4 10/11] net/smc: adapt cursor update when
sndbuf and peer DMB are merged
On 2024/3/21 16:12, Jan Karcher wrote:
>
>
> 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@...ux.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.
>
OK, will move comments above the if. Thanks!
>> + /* 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.
>
OK, will do. Thanks!
> 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,
Powered by blists - more mailing lists