Re: [PATCH net-next] net/smc: increase SMC_WR_BUF_CNT

[Dust Li <dust.li@linux.alibaba.com>]

On 2024-11-04 17:42:15, Halil Pasic wrote:
>On Thu, 31 Oct 2024 13:30:17 +0100
>Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On Sat, 26 Oct 2024 07:58:39 +0800
>> Dust Li <dust.li@linux.alibaba.com> wrote:
>> 
>> > >For some reason include/linux/wait.h does not offer a top level wrapper
>> > >macro for wait_event with interruptible, exclusive and timeout. I did
>> > >not spend too many cycles on thinking if that is even a combination that
>> > >makes sense (on the quick I don't see why not) and conversely I
>> > >refrained from making an attempt to accomplish the interruptible,
>> > >exclusive and timeout combo by using the abstraction-wise lower
>> > >level __wait_event interface.
>> > >
>> > >To alleviate the tx performance bottleneck and the CPU overhead due to
>> > >the spinlock contention, let us increase SMC_WR_BUF_CNT to 256.    
>> > 
>> > Hi,
>> > 
>> > Have you tested other values, such as 64? In our internal version, we
>> > have used 64 for some time.  
>> 
>> Yes we have, but I'm not sure the data is still to be found. Let me do
>> some digging.
>> 
>
>We did some digging and according to that data 64 is not likely to cut
>it on the TX end for highly parallel request-response workload. But we
>will measure some more these days just to be on the safe side.
>
>> > 
>> > Increasing this to 256 will require a 36K continuous physical memory
>> > allocation in smc_wr_alloc_link_mem(). Based on my experience, this may
>> > fail on servers that have been running for a long time and have
>> > fragmented memory.  
>> 
>> Good point! It is possible that I did not give sufficient thought to
>> this aspect.
>> 
>
>The failing allocation would lead to a fallback to TCP I believe. Which
>I don't consider a catastrophic failure.

Yes, but fallback to TCP may be not the only result.

When we don't have much continuous physical memory, allocating a large
continuous physical memory without flags like __GFP_NORETRY would cause
memory compaction. We've encounter problems before, the one I still
remember is the statistics buffer for mlx5 was once allocated using
kmalloc, and it was changed to kvmalloc later because of the large
physical continous memory allocation cause problems with online servers.

>
>But let us put this patch on hold and see if we can come up with
>something better.

Agree

Best regards,
Dust