| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <51015767-e6bd-4ff0-9904-8a643d209e44@linux.ibm.com>
Date: Wed, 24 Sep 2025 22:58:42 +0530
From: Sidraya Jayagond <sidraya@...ux.ibm.com>
To: Halil Pasic <pasic@...ux.ibm.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
"D. Wythe" <alibuda@...ux.alibaba.com>,
Dust Li <dust.li@...ux.alibaba.com>,
Wenjia Zhang <wenjia@...ux.ibm.com>,
Mahanta Jambigi
<mjambigi@...ux.ibm.com>,
Tony Lu <tonylu@...ux.alibaba.com>, Wen Gu <guwen@...ux.alibaba.com>,
netdev@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-rdma@...r.kernel.org,
linux-s390@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/2] net/smc: handle -ENOMEM from
smc_wr_alloc_link_mem gracefully
On 22/09/25 3:14 am, Halil Pasic wrote:
> Currently if a -ENOMEM from smc_wr_alloc_link_mem() is handled by
> giving up and going the way of a TCP fallback. This was reasonable
> before the sizes of the allocations there were compile time constants
> and reasonably small. But now those are actually configurable.
>
> So instead of giving up, keep retrying with half of the requested
> size unless we dip below the old static sizes -- then give up!
>
> Signed-off-by: Halil Pasic <pasic@...ux.ibm.com>
> Reviewed-by: Wenjia Zhang <wenjia@...ux.ibm.com>
> Reviewed-by: Mahanta Jambigi <mjambigi@...ux.ibm.com>
> ---
> Documentation/networking/smc-sysctl.rst | 8 ++++--
> net/smc/smc_core.c | 34 +++++++++++++++++--------
> net/smc/smc_core.h | 2 ++
> net/smc/smc_wr.c | 28 ++++++++++----------
> 4 files changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/networking/smc-sysctl.rst b/Documentation/networking/smc-sysctl.rst
> index c94d750c7c84..c8dbe7ac8bdf 100644
> --- a/Documentation/networking/smc-sysctl.rst
> +++ b/Documentation/networking/smc-sysctl.rst
> @@ -85,7 +85,9 @@ smcr_max_send_wr - INTEGER
>
> 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
> + of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails, we keep retrying
> + with half of the buffer count until it is ether successful or (unlikely)
> + we dip below the old hard coded value which is 16 where we give up much
> like before having this control.
>
> Default: 16
> @@ -103,7 +105,9 @@ smcr_max_recv_wr - INTEGER
>
> 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
> + of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails, we keep retrying
> + with half of the buffer count until it is ether successful or (unlikely)
> + we dip below the old hard coded value which is 16 where we give up much
> like before having this control.
>
> Default: 48
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 2a559a98541c..f8131b4dfcd6 100644
> --- 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;
> lnk->lgr = lgr;
> smc_lgr_hold(lgr); /* lgr_put in smcr_link_clear() */
> lnk->link_idx = link_idx;
> @@ -836,27 +838,39 @@ int smcr_link_init(struct smc_link_group *lgr, struct smc_link *lnk,
> rc = smc_llc_link_init(lnk);
> if (rc)
> goto out;
> - rc = smc_wr_alloc_link_mem(lnk);
> - if (rc)
> - goto clear_llc_lnk;
> rc = smc_ib_create_protection_domain(lnk);
> if (rc)
> - goto free_link_mem;
> - rc = smc_ib_create_queue_pair(lnk);
> - if (rc)
> - goto dealloc_pd;
> + goto clear_llc_lnk;
> + do {
> + rc = smc_ib_create_queue_pair(lnk);
> + if (rc)
> + goto dealloc_pd;
> + rc = smc_wr_alloc_link_mem(lnk);
> + if (!rc)
> + break;
> + else if (rc != -ENOMEM) /* give up */
> + goto destroy_qp;
> + /* retry with smaller ... */
> + lnk->max_send_wr /= 2;
> + lnk->max_recv_wr /= 2;
> + /* ... unless droping below old SMC_WR_BUF_SIZE */
> + if (lnk->max_send_wr < 16 || lnk->max_recv_wr < 48)
> + goto destroy_qp;
> + smc_ib_destroy_queue_pair(lnk);
> + } while (1);
> +
> rc = smc_wr_create_link(lnk);
> if (rc)
> - goto destroy_qp;
> + goto free_link_mem;
> lnk->state = SMC_LNK_ACTIVATING;
> return 0;
>
> +free_link_mem:
> + smc_wr_free_link_mem(lnk);
> destroy_qp:
> smc_ib_destroy_queue_pair(lnk);
> dealloc_pd:
> smc_ib_dealloc_protection_domain(lnk);
> -free_link_mem:
> - smc_wr_free_link_mem(lnk);
> clear_llc_lnk:
> smc_llc_link_clear(lnk, false);
> out:
> diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
> index ab2d15929cb2..b9addf633d8a 100644
> --- a/net/smc/smc_core.h
> +++ b/net/smc/smc_core.h
> @@ -174,6 +174,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;
> };
>
> /* For now we just allow one parallel link per link group. The SMC protocol
> diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
> index f5b2772414fd..6b14711f0c93 100644
> --- a/net/smc/smc_wr.c
> +++ b/net/smc/smc_wr.c
> @@ -548,9 +548,9 @@ void smc_wr_remember_qp_attr(struct smc_link *lnk)
> IB_QP_DEST_QPN,
> &init_attr);
>
> - lnk->wr_tx_cnt = min_t(size_t, lnk->lgr->max_send_wr,
> + lnk->wr_tx_cnt = min_t(size_t, lnk->max_send_wr,
> lnk->qp_attr.cap.max_send_wr);
> - lnk->wr_rx_cnt = min_t(size_t, lnk->lgr->max_recv_wr,
> + lnk->wr_rx_cnt = min_t(size_t, lnk->max_recv_wr,
> lnk->qp_attr.cap.max_recv_wr);
> }
>
> @@ -742,51 +742,51 @@ int smc_wr_alloc_lgr_mem(struct smc_link_group *lgr)
> int smc_wr_alloc_link_mem(struct smc_link *link)
> {
> /* allocate link related memory */
> - link->wr_tx_bufs = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_bufs = kcalloc(link->max_send_wr,
> SMC_WR_BUF_SIZE, GFP_KERNEL);
> if (!link->wr_tx_bufs)
> goto no_mem;
> - link->wr_rx_bufs = kcalloc(link->lgr->max_recv_wr, link->wr_rx_buflen,
> + link->wr_rx_bufs = kcalloc(link->max_recv_wr, link->wr_rx_buflen,
> GFP_KERNEL);
> if (!link->wr_rx_bufs)
> goto no_mem_wr_tx_bufs;
> - link->wr_tx_ibs = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_ibs = kcalloc(link->max_send_wr,
> sizeof(link->wr_tx_ibs[0]), GFP_KERNEL);
> if (!link->wr_tx_ibs)
> goto no_mem_wr_rx_bufs;
> - link->wr_rx_ibs = kcalloc(link->lgr->max_recv_wr,
> + link->wr_rx_ibs = kcalloc(link->max_recv_wr,
> sizeof(link->wr_rx_ibs[0]),
> GFP_KERNEL);
> if (!link->wr_rx_ibs)
> goto no_mem_wr_tx_ibs;
> - link->wr_tx_rdmas = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_rdmas = kcalloc(link->max_send_wr,
> sizeof(link->wr_tx_rdmas[0]),
> GFP_KERNEL);
> if (!link->wr_tx_rdmas)
> goto no_mem_wr_rx_ibs;
> - link->wr_tx_rdma_sges = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_rdma_sges = kcalloc(link->max_send_wr,
> sizeof(link->wr_tx_rdma_sges[0]),
> GFP_KERNEL);
> if (!link->wr_tx_rdma_sges)
> goto no_mem_wr_tx_rdmas;
> - link->wr_tx_sges = kcalloc(link->lgr->max_send_wr, sizeof(link->wr_tx_sges[0]),
> + link->wr_tx_sges = kcalloc(link->max_send_wr, sizeof(link->wr_tx_sges[0]),
> GFP_KERNEL);
> if (!link->wr_tx_sges)
> goto no_mem_wr_tx_rdma_sges;
> - link->wr_rx_sges = kcalloc(link->lgr->max_recv_wr,
> + link->wr_rx_sges = kcalloc(link->max_recv_wr,
> sizeof(link->wr_rx_sges[0]) * link->wr_rx_sge_cnt,
> GFP_KERNEL);
> if (!link->wr_rx_sges)
> goto no_mem_wr_tx_sges;
> - link->wr_tx_mask = bitmap_zalloc(link->lgr->max_send_wr, GFP_KERNEL);
> + link->wr_tx_mask = bitmap_zalloc(link->max_send_wr, GFP_KERNEL);
> if (!link->wr_tx_mask)
> goto no_mem_wr_rx_sges;
> - link->wr_tx_pends = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_pends = kcalloc(link->max_send_wr,
> sizeof(link->wr_tx_pends[0]),
> GFP_KERNEL);
> if (!link->wr_tx_pends)
> goto no_mem_wr_tx_mask;
> - link->wr_tx_compl = kcalloc(link->lgr->max_send_wr,
> + link->wr_tx_compl = kcalloc(link->max_send_wr,
> sizeof(link->wr_tx_compl[0]),
> GFP_KERNEL);
> if (!link->wr_tx_compl)
> @@ -907,7 +907,7 @@ int smc_wr_create_link(struct smc_link *lnk)
> goto dma_unmap;
> }
> smc_wr_init_sge(lnk);
> - bitmap_zero(lnk->wr_tx_mask, lnk->lgr->max_send_wr);
> + bitmap_zero(lnk->wr_tx_mask, lnk->max_send_wr);
> init_waitqueue_head(&lnk->wr_tx_wait);
> rc = percpu_ref_init(&lnk->wr_tx_refs, smcr_wr_tx_refs_free, 0, GFP_KERNEL);
> if (rc)
Code changes looks good to me.
Reviewed-by: Sidraya Jayagond <sidraya@...ux.ibm.com>
Powered by blists - more mailing lists