Received-SPF: pass (google.com: domain of linux-kernel+bounces-146942-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <eb7eb42a-ea96-41d9-b385-c459379bed99@linux.alibaba.com>
Date: Tue, 16 Apr 2024 22:06:04 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v6 10/11] net/smc: adapt cursor update when
 sndbuf and peer DMB are merged
To: Simon Horman <horms@kernel.org>
Cc: wintera@linux.ibm.com, twinkler@linux.ibm.com, hca@linux.ibm.com,
 gor@linux.ibm.com, agordeev@linux.ibm.com, davem@davemloft.net,
 edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
 wenjia@linux.ibm.com, jaka@linux.ibm.com, borntraeger@linux.ibm.com,
 svens@linux.ibm.com, alibuda@linux.alibaba.com, tonylu@linux.alibaba.com,
 linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
 netdev@vger.kernel.org
References: <20240414040304.54255-1-guwen@linux.alibaba.com>
 <20240414040304.54255-11-guwen@linux.alibaba.com>
 <20240416110538.GK2320920@kernel.org>
From: Wen Gu <guwen@linux.alibaba.com>
In-Reply-To: <20240416110538.GK2320920@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 2024/4/16 19:05, Simon Horman wrote:
> On Sun, Apr 14, 2024 at 12:03:03PM +0800, Wen Gu wrote:
>> If the local 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>
> 
> ...
> 
>> @@ -255,6 +256,14 @@ 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;
>> +
>> +	if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
>> +		/* if local sndbuf shares the same memory region with
>> +		 * peer DMB, then don't update the tx_curs_fin
>> +		 * and sndbuf_space until peer has consumed the data.
>> +		 */
>> +		return rc;
> 
> Hi Wen Gu,
> 
> A minor nit from my side:
> 
> To my mind "return rc" implies returning an error value.
> But here rc is 0, which based on the comment seems correct.
> So perhaps it would be clearer to simply return 0.
> 
> Flagged by Smatch.
> 
>> +
>>   	/* 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);
> 
> ...

OK. I will improve it and another 'return rc' at the end of
smcd_cdc_msg_send(). Thanks! Simon.