Received-SPF: pass (google.com: domain of linux-kernel+bounces-78514-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <b8ad3e35-7dba-41f2-a301-65f8162a2dbd@linux.ibm.com>
Date: Fri, 23 Feb 2024 15:42:10 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next 14/15] net/smc: introduce loopback-ism DMB data
 copy control
Content-Language: en-GB
To: Wen Gu <guwen@linux.alibaba.com>, wintera@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,
        jaka@linux.ibm.com, Gerd Bayer <gbayer@linux.ibm.com>
Cc: borntraeger@linux.ibm.com, svens@linux.ibm.com, alibuda@linux.alibaba.com,
        tonylu@linux.alibaba.com, linux-s390@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240111120036.109903-1-guwen@linux.alibaba.com>
 <20240111120036.109903-15-guwen@linux.alibaba.com>
 <b3b71f26-239f-49c9-98e8-7eba2c4ecf69@linux.ibm.com>
 <58f36cb7-7427-4ed7-9a8e-baaacdd774cb@linux.alibaba.com>
From: Wenjia Zhang <wenjia@linux.ibm.com>
In-Reply-To: <58f36cb7-7427-4ed7-9a8e-baaacdd774cb@linux.alibaba.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0


On 20.02.24 04:36, Wen Gu wrote:
> 
> 
> On 2024/2/16 22:25, Wenjia Zhang wrote:
>>
>>
>> On 11.01.24 13:00, Wen Gu wrote:
>>> This provides a way to {get|set} whether loopback-ism device supports
>>> merging sndbuf with peer DMB to eliminate data copies between them.
>>>
>>> echo 0 > /sys/devices/virtual/smc/loopback-ism/dmb_copy # support
>>> echo 1 > /sys/devices/virtual/smc/loopback-ism/dmb_copy # not support
>>>
>> Besides the same confusing as Niklas already mentioned, the name of 
>> the option looks not clear enough to what it means. What about:
>> echo 1 > /sys/devices/virtual/smc/loopback-ism/nocopy_support # merge 
>> mode
>> echo 0 > /sys/devices/virtual/smc/loopback-ism/nocopy_support # copy mode
>>
> 
> OK, if we decide to keep the knobs, I will improve the name. Thanks!
> 
>>> The settings take effect after re-activating loopback-ism by:
>>>
>>> echo 0 > /sys/devices/virtual/smc/loopback-ism/active
>>> echo 1 > /sys/devices/virtual/smc/loopback-ism/active
>>>
>>> After this, the link group related to loopback-ism will be flushed and
>>> the sndbufs of subsequent connections will be merged or not merged with
>>> peer DMB.
>>>
>>> The motivation of this control is that the bandwidth will be highly
>>> improved when sndbuf and DMB are merged, but when virtually contiguous
>>> DMB is provided and merged with sndbuf, it will be concurrently accessed
>>> on Tx and Rx, then there will be a bottleneck caused by lock contention
>>> of find_vmap_area when there are many CPUs and CONFIG_HARDENED_USERCOPY
>>> is set (see link below). So an option is provided.
>>>
>>> Link: 
>>> https://lore.kernel.org/all/238e63cd-e0e8-4fbf-852f-bc4d5bc35d5a@linux.alibaba.com/
>>> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
>>> ---
>> We tried some simple workloads, and the performance of the no-copy 
>> case was remarkable. Thus, we're wondering if it is necessary to have 
>> the tunable setting in this loopback case? Or rather, why do we need 
>> the copy option? Is that because of the bottleneck caused by using the 
>> combination of the no-copy and virtually contiguours DMA? Or at least 
>> let no-copy as the default one.
> 
> Yes, it is because the bottleneck caused by using the combination of the 
> no-copy
> and virtual-DMB. If we have to use virtual-DMB and 
> CONFIG_HARDENED_USERCOPY is
> set, then we may be forced to use copy mode in many CPUs environment, to 
> get the
> good latency performance (the bandwidth performance still drop because 
> of copy mode).
> 
> But if we agree that physical-DMB is acceptable (it costs 1 physical 
> buffer per conn side
> in loopback-ism no-copy mode, same as what sndbuf costs when using s390 
> ISM), then
> there is no such performance issue and the two knobs can be removed. 
> (see also the reply
> for 13/15 patch [1]).
> 
> [1] 
> https://lore.kernel.org/netdev/442061eb-107a-421d-bc2e-13c8defb0f7b@linux.alibaba.com/
> 
> Thanks!
Thank you, Wen, for the elaboration! As I said, though we did see some 
better performance on using the virtually contiguous memory with a 
simple test, the improvement was not really significant. Additionally, 
our environment ist very different as your 48 CPUs qemu environment, and 
it also depends on the workload. I think I can understand why you see 
better performance by using physically contiguous memory. Anyway, I 
don't have any objection on using physical-DMB only. But I still want to 
see if there is any other opinion.