smc_wr_tx_put_slot() tries to reset the v2 pending slot and buffer with memset(&link->wr_tx_v2_pend, 0, sizeof(link->wr_tx_v2_pend)) and the equivalent for wr_tx_buf_v2. Both are pointers, so this zeroes the 8-byte pointer variable instead of the structure it points to. The pending slot and buffer are therefore never actually cleared, and the pointers get overwritten with NULL. Pass the pointers directly and use sizeof(*pointer) so the intended structures are cleared. Fixes: 8799e310fb3f ("net/smc: add v2 support to the work request layer") Signed-off-by: D. Wythe Reviewed-by: Dust Li Reviewed-by: Mahanta Jambigi --- net/smc/smc_wr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c index 97ba46893b17..def2ab84b0c7 100644 --- a/net/smc/smc_wr.c +++ b/net/smc/smc_wr.c @@ -288,10 +288,10 @@ int smc_wr_tx_put_slot(struct smc_link *link, } else if (link->lgr->smc_version == SMC_V2 && pend->idx == link->wr_tx_cnt) { /* Large v2 buffer */ - memset(&link->wr_tx_v2_pend, 0, - sizeof(link->wr_tx_v2_pend)); - memset(&link->lgr->wr_tx_buf_v2, 0, - sizeof(link->lgr->wr_tx_buf_v2)); + memset(link->wr_tx_v2_pend, 0, + sizeof(*link->wr_tx_v2_pend)); + memset(link->lgr->wr_tx_buf_v2, 0, + sizeof(*link->lgr->wr_tx_buf_v2)); return 1; } -- 2.45.0 smc_wr_tx_get_free_slot() waits for a free TX slot with wait_event_interruptible_timeout(). Since the wait_event family enqueues waiters as non-exclusive, wake_up() may wake multiple waiters even though only one can use the slot, causing thundering-herd contention when slots are scarce. Use an exclusive wait loop with prepare_to_wait_exclusive() so wake_up() wakes only one waiter per freed slot. smc_wr_wakeup_tx_wait() still uses wake_up_all() during link teardown, so teardown behavior is unchanged. This also corrects the return value on a pending signal: the previous wait_event_interruptible_timeout() path fell through to the "no free slot" case and returned -EPIPE, masking the signal as a connection error. The open-coded loop now returns -ERESTARTSYS, matching the standard interruptible-wait semantics and letting the syscall restart machinery handle it. Performance =========== Measured with uperf between two peers over SMC-R. The benefit depends on how often the TX slot wait path is actually taken. With the default settings, where many connections share a link group and the send queue is small, slots are scarce and the wait path is hot: net.smc.smcr_max_conns_per_lgr = 255 net.smc.smcr_max_send_wr = 16 net.smc.smcr_max_recv_wr = 48 workload baseline patched delta --------------------------------------------------------- rr1c-200x1000-50.xml 655.06 Mb/s 1.53 Gb/s +134% rr1c-1x1-250.xml 371.03 Kb/s 2.07 Mb/s +458% With a tuned configuration, where slots are mostly available and the wait path is rarely entered, the change is neutral to slightly positive: net.smc.smcr_max_conns_per_lgr = 32 net.smc.smcr_max_send_wr = 64 net.smc.smcr_max_recv_wr = 64 workload baseline patched delta --------------------------------------------------------- rr1c-200x1000-50.xml 1.74 Gb/s 1.80 Gb/s +3.4% rr1c-1x1-250.xml 3.11 Mb/s 3.14 Mb/s +1.0% So the change does not regress the uncontended case, and recovers most of the throughput lost to thundering-herd wakeups once slots become scarce. Signed-off-by: D. Wythe Reviewed-by: Wen Gu Reviewed-by: Mahanta Jambigi --- net/smc/smc_wr.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c index def2ab84b0c7..64413008f9c1 100644 --- a/net/smc/smc_wr.c +++ b/net/smc/smc_wr.c @@ -198,11 +198,13 @@ int smc_wr_tx_get_free_slot(struct smc_link *link, struct smc_rdma_wr **wr_rdma_buf, struct smc_wr_tx_pend_priv **wr_pend_priv) { + unsigned long timeout = SMC_WR_TX_WAIT_FREE_SLOT_TIME; struct smc_link_group *lgr = smc_get_lgr(link); struct smc_wr_tx_pend *wr_pend; u32 idx = link->wr_tx_cnt; struct ib_send_wr *wr_ib; u64 wr_id; + DEFINE_WAIT(wait); int rc; *wr_buf = NULL; @@ -212,17 +214,31 @@ int smc_wr_tx_get_free_slot(struct smc_link *link, if (rc) return rc; } else { - rc = wait_event_interruptible_timeout( - link->wr_tx_wait, - !smc_link_sendable(link) || - lgr->terminating || - (smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY), - SMC_WR_TX_WAIT_FREE_SLOT_TIME); - if (!rc) { - /* timeout - terminate link */ - smcr_link_down_cond_sched(link); - return -EPIPE; + rc = 0; + for (;;) { + prepare_to_wait_exclusive(&link->wr_tx_wait, &wait, + TASK_INTERRUPTIBLE); + if (!smc_link_sendable(link) || lgr->terminating || + smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY) + break; + timeout = schedule_timeout(timeout); + /* re-check */ + if (!smc_link_sendable(link) || lgr->terminating || + smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY) + break; + if (!timeout) { + /* timeout - terminate link */ + smcr_link_down_cond_sched(link); + break; + } + if (signal_pending(current)) { + rc = -ERESTARTSYS; + break; + } } + finish_wait(&link->wr_tx_wait, &wait); + if (rc) + return rc; if (idx == link->wr_tx_cnt) return -EPIPE; } -- 2.45.0