Subject: Re: [PATCH net-next v5 2/2] net/smc: handle -ENOMEM from smc_wr_alloc_link_mem gracefully

From: Halil Pasic <[email-masked]>
Date: Wed, 1 Oct 2025 11:37:51 +0200

On Wed, 1 Oct 2025 09:21:09 +0200
Paolo Abeni <[email-masked]> wrote:

> >>
> >> Since in Alibaba we doesn't use multi-link configurations, we haven't tested
> >> this scenario. Have you tested the link-down handling process in a multi-link
> >> setup?
> >>  
> > 
> > Mahanta was so kind to do most of the testing on this. I don't think
> > I've tested this myself. @Mahanta: Would you be kind to give this a try
> > if it wasn't covered in the past? The best way is probably to modify
> > the code to force such a scenario. I don't think it is easy to somehow
> > trigger in the wild.
> > 
> > BTW I don't expect any problems. I think at worst the one link would
> > end up giving worse performance than the other, but I guess that can
> > happen for other reasons as well (like different HW for the two links).
> > 
> > But I think getting some sort of a query interface which would tell
> > us how much did we end up with down the road would be a good idea anyway.
> > 
> > And I hope we can switch to vmalloc down the road as well, which would
> > make back off less likely.  
> 
> Unfortunately we are closing the net-next PR right now and I would
> prefer such testing being reported explicitly. Let's defer this series
> to the next cycle: please re-post when net-next will reopen after Oct 12th.

Than you Paolo! Will do! I have talked to Mahanta yesterday about this,
and he has done some testing in the meanwhile, but I'm not sure he
covered everything he wanted. And he is out for the week (today and
tomorrow is public holiday in his geography).

Regards,
Halil

---

From: Bagas Sanjaya <[email-masked]>
Date: Wed, 1 Oct 2025 17:29:59 +0700

On Mon, Sep 29, 2025 at 02:00:00AM +0200, Halil Pasic wrote:
> diff --git a/Documentation/networking/smc-sysctl.rst b/Documentation/networking/smc-sysctl.rst
> index a874d007f2db..5de4893ef3e7 100644
> --- a/Documentation/networking/smc-sysctl.rst
> +++ b/Documentation/networking/smc-sysctl.rst
> @@ -71,3 +71,39 @@ smcr_max_conns_per_lgr - INTEGER
>  	acceptable value ranges from 16 to 255. Only for SMC-R v2.1 and later.
>  
>  	Default: 255
> +
> +smcr_max_send_wr - INTEGER
> +	So called work request buffers are SMCR link (and RDMA queue pair) level
        So-called
> +	resources necessary for performing RDMA operations. Since up to 255
> +	connections can share a link group and thus also a link and the number
> +	of the work request buffers is decided when the link is allocated,
> +	depending on the workload it can be a bottleneck in a sense that threads
> +	have to wait for work request buffers to become available. Before the
> +	introduction of this control the maximal number of work request buffers
> +	available on the send path used to be hard coded to 16. With this control
> +	it becomes configurable. The acceptable range is between 2 and 2048.
> +
> +	Please be aware that all the buffers need to be allocated as a physically
> +	continuous array in which each element is a single buffer and has the size
> +	of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails we give up much
> +	like before having this control.
> +
> +	Default: 16
> +
> +smcr_max_recv_wr - INTEGER
> +	So called work request buffers are SMCR link (and RDMA queue pair) level
Ditto.
> +	resources necessary for performing RDMA operations. Since up to 255
> +	connections can share a link group and thus also a link and the number
> +	of the work request buffers is decided when the link is allocated,
> +	depending on the workload it can be a bottleneck in a sense that threads
> +	have to wait for work request buffers to become available. Before the
> +	introduction of this control the maximal number of work request buffers
> +	available on the receive path used to be hard coded to 16. With this control
> +	it becomes configurable. The acceptable range is between 2 and 2048.
> +
> +	Please be aware that all the buffers need to be allocated as a physically
> +	continuous array in which each element is a single buffer and has the size
> +	of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails we give up much
> +	like before having this control.
> +
> +	Default: 48

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

---

From: Paolo Abeni <[email-masked]>
Date: Wed, 1 Oct 2025 09:21:09 +0200

On 9/29/25 11:22 AM, Halil Pasic wrote:
> On Mon, 29 Sep 2025 09:50:52 +0800
> Dust Li <[email-masked]> wrote:
> 
>>> @@ -175,6 +175,8 @@ struct smc_link {
>>> 	struct completion	llc_testlink_resp; /* wait for rx of testlink */
>>> 	int			llc_testlink_time; /* testlink interval */
>>> 	atomic_t		conn_cnt; /* connections on this link */
>>> +	u16			max_send_wr;
>>> +	u16			max_recv_wr;  
>>
>> Here, you've moved max_send_wr/max_recv_wr from the link group to individual links.
>> This means we can now have different max_send_wr/max_recv_wr values on two
>> different links within the same link group.
> 
> Only if allocations fail. Please notice that the hunk:
> 
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -810,6 +810,8 @@ int smcr_link_init(struct smc_link_group *lgr, struct smc_link *lnk,
>  	lnk->clearing = 0;
>  	lnk->path_mtu = lnk->smcibdev->pattr[lnk->ibport - 1].active_mtu;
>  	lnk->link_id = smcr_next_link_id(lgr);
> +	lnk->max_send_wr = lgr->max_send_wr;
> +	lnk->max_recv_wr = lgr->max_recv_wr;
> 
> initializes the link values with the values from the lgr which are in
> turn picked up form the systctls at lgr creation time. I have made an
> effort to keep these values the same for each link, but in case the
> allocation fails and we do back off, we can end up with different values
> on the links. 
> 
> The alternative would be to throw in the towel, and not create
> a second link if we can't match what worked for the first one.
> 
>>

Thanks,

Paolo