[PATCH net-next 0/2] net/smc: fix v2 slot clearing and reduce TX slot contention

This series contains the two reviewed patches from the previously posted
"net/smc: transition to RDMA core CQ pooling" series (v4), reposted as a
standalone series so they can land independently. The remaining CQ pooling
patch will be reposted separately after these two are merged.

Patch 1 fixes smc_wr_tx_put_slot() to clear the v2 pending slot and buffer
structures instead of the pointer variables, the memset targets were the
8-byte pointers themselves so the structures were never actually cleared.
Patch 2 reduces TX slot contention by switching TX slot allocation from
non-exclusive wait_event() to prepare_to_wait_exclusive(), avoiding
thundering-herd wakes when slots are scarce.

For patch 2, uperf numbers are now included in the commit message, as
requested by Mahanta during v1 review. The short version is that the
gain tracks how often the TX slot wait path is actually taken: with the
default sysctl settings, where a link group multiplexes many connections
over a small send queue, throughput improves by 134% on the 200x1000
request/response workload and by 458% on the 1-byte ping-pong workload;
with a tuned configuration where slots are rarely exhausted, the change
is neutral (+3.4% and +1.0%, i.e. within noise). See patch 2 for the
full table.

Link: https://lore.kernel.org/netdev/20260721175309.321b6503@kernel.org/
Link: https://lore.kernel.org/netdev/20260716113745.65234-1-alibuda@linux.alibaba.com/
Link: https://lore.kernel.org/netdev/20260821091702.21458-2-alibuda@linux.alibaba.com/

D. Wythe (2):
  net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()
  net/smc: reduce TX slot contention with exclusive wait

 net/smc/smc_wr.c | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

-- 
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 <alibuda@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
 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

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 <alibuda@linux.alibaba.com>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
 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

On 10/09/26 4:14 pm, D. Wythe wrote:

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 <alibuda@linux.alibaba.com>
Reviewed-by: Wen Gu <guwen@linux.alibaba.com>
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>

I don't see a changelog describing what has changed since the previous
version. Any Sashiko comments were addressed? Could you explain why this
is being reposted?

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/smc: reduce TX slot contention with exclusive wait

This replaces the wait_event_interruptible_timeout() in
smc_wr_tx_get_free_slot() with an open-coded loop built on
prepare_to_wait_exclusive(), so a wake_up() on link->wr_tx_wait only wakes
one waiter per freed TX slot.  It also changes the return value on a
pending signal from -EPIPE to -ERESTARTSYS.

A few questions below about the new wait loop and about the changed return
code.

The commit message says:

  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.

[Severity: Low]

Should this part carry a Fixes: tag?  git blame on the baseline points the
wait_event_interruptible_timeout() call and the timeout-only "if (!rc)"
handling at f38ba179c6ca ("smc: work request (WR) base for use by LLC and
CDC"), with later touches in 15e1b99aadfb27 and 87523930a16eb5.  The
corrected value is user visible via smc_shutdown() -> smc_close_active()
-> smc_close_final() -> smc_wr_tx_get_free_slot().

Also, would it make sense to split the return-code correction from the
wait-queue rework?  As it stands both live in the same hunk, so the
corrected error code cannot be backported without the performance change.

[Severity: Medium]

Can a freed TX slot end up unclaimed while another waiter keeps sleeping?

prepare_to_wait_exclusive() queues the entry with WQ_FLAG_EXCLUSIVE and sets
TASK_INTERRUPTIBLE before the condition is evaluated, and the condition here
is not cheap - smc_wr_tx_get_free_slot_index() scans up to wr_tx_cnt bits:

net/smc/smc_wr.c:smc_wr_tx_get_free_slot_index() {
	...
	for_each_clear_bit(*idx, link->wr_tx_mask, link->wr_tx_cnt) {
		if (!test_and_set_bit(*idx, link->wr_tx_mask))
			return 0;
	}
	*idx = link->wr_tx_cnt;
	return -EBUSY;
}

Both slot producers use plain wake_up(), which is nr_exclusive = 1, so
exactly one exclusive waiter is woken per freed slot:

net/smc/smc_wr.c:smc_wr_tx_process_cqe() {
	...
	wake_up(&link->wr_tx_wait);
}

net/smc/smc_wr.c:smc_wr_tx_put_slot() {
	...
	test_and_clear_bit(idx, link->wr_tx_mask);
	wake_up(&link->wr_tx_wait);
}

The interleaving I have in mind:

  1) all slots busy, waiter W sleeps on link->wr_tx_wait
  2) a CQE frees slot i, wake_up() wakes W, autoremove_wake_function()
     dequeues W, the credit is consumed
  3) before W runs, a fresh process-context caller T enters the loop,
     prepare_to_wait_exclusive() queues T, T's first scan takes slot i
     (a bare test_and_set_bit(), no reservation) and T breaks out
  4) W re-checks, finds nothing free, requeues and sleeps again
  5) slot j is freed, wake_up() walks the queue, reaches T first (still
     queued and TASK_INTERRUPTIBLE), T absorbs the credit and returns

Slot j is now free while W sleeps.  Since the loop never issues another
wake_up(&link->wr_tx_wait) when it exits without consuming the announced
slot, is that credit simply dropped by finish_wait()?  Would a
wake_up(&link->wr_tx_wait) on the paths that leave the queue without
claiming the announced slot be appropriate here?

With the previous non-exclusive wait_event_interruptible_timeout(), every
wake_up() re-evaluated the condition for all waiters, so a free slot could
not coexist with sleeping waiters.

Related to the above: if a waiter misses its wakeup and no further slot is
released within SMC_WR_TX_WAIT_FREE_SLOT_TIME, does it reach this branch and
call smcr_link_down_cond_sched(), tearing down the link for the whole link
group even though a TX slot was actually available?

[Severity: Medium]

Is returning raw -ERESTARTSYS to all callers of
smc_wr_tx_get_free_slot() safe?  The sibling blocking helper in the same
file converts it:

net/smc/smc_wr.c:smc_wr_reg_send() {
	...
	if (rc == -ERESTARTSYS)
		return -EINTR;
	...
}

The path I traced is shutdown(fd, SHUT_RDWR) on an SMC_ACTIVE socket:

  smc_shutdown() -> smc_close_active() -> smc_close_final() ->
  smc_cdc_get_slot_and_msg_send() -> smc_cdc_get_free_slot() ->
  smc_wr_tx_get_free_slot()

smc_close_active() advances the state machine before it looks at the return
value, setting local_tx_ctrl.conn_state_flags.peer_conn_closed = 1 and
sk->sk_state = SMC_PEERCLOSEWAIT1.  With all TX slots busy and a signal
pending, -ERESTARTSYS now reaches smc_shutdown(), which returns it verbatim.

With SA_RESTART the syscall is restarted transparently, re-enters
smc_shutdown() in state SMC_PEERCLOSEWAIT1 where smc_close_sent_any_close()
is already true, so nothing is sent and shutdown() returns 0.  Does this
report success to the application although the SMC close message was never
transmitted?  The baseline returned -EPIPE here, so the failure was visible.

The commit message states the new value is "matching the standard
interruptible-wait semantics and letting the syscall restart machinery
handle it" - does that hold for a call site that has already committed side
effects before the error is returned?

Other callers only special-case -EBUSY, for example:

net/smc/smc_tx.c:smcr_tx_sndbuf_nonempty() {
	...
	rc = smc_cdc_get_free_slot(conn, link, &wr_buf, &wr_rdma_buf, &pend);
	if (rc < 0) {
		smc_wr_tx_link_put(link);
		if (rc == -EBUSY) { ... }
		return rc;
	}
	...
}

so -ERESTARTSYS is propagated as a hard error with no retry queued.

For completeness on what I could not confirm: on the connect path every
error is converted to an SMC_CLC_DECL_* code before reaching
"smc->sk.sk_err = -rc", so -ERESTARTSYS does not appear to leak out through
SO_ERROR or poll, and I could not demonstrate signal_pending() becoming true
inside workqueue workers.

[ ... ]