[PATCH net-next v4 0/2] net/rds: RDS-TCP bug fix collection, subset 1: Work queue scalability

[PATCH net-next v4 0/2] net/rds: RDS-TCP bug fix collection, subset 1: Work queue scalability

[Allison Henderson]

From: Allison Henderson <allison.henderson@oracle.com>

Hi all,

This is subset 1 of the RDS-TCP bug fix collection series I posted last
Oct.  The greater series aims to correct multiple rds-tcp bugs that
can cause dropped or out of sequence messages.  The set was starting to
get a bit large, so I've broken it down into smaller sets to make
reviews more manageable.

In this subset, we focus on work queue scalability.  Messages queues
are refactored to operate in parallel across multiple connections,
which improves response times and avoids timeouts.

The entire set can be viewed in the rfc here:
https://lore.kernel.org/netdev/20251022191715.157755-1-achender@kernel.org/

Questions, comments, flames appreciated!
Thanks!
Allison

Change Log:
rfc->v1
 - Fixed lkp warnings and white space cleanup
 - Split out the workqueue changes as a subset

v1->v2
 [PATCH 1/2] net/rds: Add per cp work queue
   - Checkpatch nits
 [PATCH 2/2] net/rds: Give each connection its own workqueue
   - Checkpatch nits
   - Updated commit message with workqueue overhead accounting

v2->v3
 [PATCH 2/2] net/rds: Give each connection path its own workqueue
   - Updated commit message with worst case connection path accounting
   - Use rds_wq as a fall back if queue alloc fails
   - Checkpatch nits 

v3->v4
  [PATCH 2/2] net/rds: Give each connection path its own workqueue
   - Fixed memleak warning in __rds_conn_create error path

Allison Henderson (2):
  net/rds: Add per cp work queue
  net/rds: Give each connection path its own workqueue

 net/rds/connection.c | 17 +++++++++++++++--
 net/rds/ib_recv.c    |  2 +-
 net/rds/ib_send.c    |  2 +-
 net/rds/rds.h        |  1 +
 net/rds/send.c       |  9 +++++----
 net/rds/tcp_recv.c   |  2 +-
 net/rds/tcp_send.c   |  2 +-
 net/rds/threads.c    | 16 ++++++++--------
 8 files changed, 33 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH net-next v4 1/2] net/rds: Add per cp work queue

[Allison Henderson]

From: Allison Henderson <allison.henderson@oracle.com>

This patch adds a per connection workqueue which can be initialized
and used independently of the globally shared rds_wq.

This patch is the first in a series that aims to address tcp ack
timeouts during the tcp socket shutdown sequence.

This initial refactoring lays the ground work needed to alleviate
queue congestion during heavy reads and writes.  The independently
managed queues will allow shutdowns and reconnects respond more quickly
before the peer(s) timeout waiting for the proper acks.

Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
 net/rds/connection.c |  5 +++--
 net/rds/ib_recv.c    |  2 +-
 net/rds/ib_send.c    |  2 +-
 net/rds/rds.h        |  1 +
 net/rds/send.c       |  9 +++++----
 net/rds/tcp_recv.c   |  2 +-
 net/rds/tcp_send.c   |  2 +-
 net/rds/threads.c    | 16 ++++++++--------
 8 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index 68bc88cce84ec..dc7323707f450 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -269,6 +269,7 @@ static struct rds_connection *__rds_conn_create(struct net *net,
 		__rds_conn_path_init(conn, &conn->c_path[i],
 				     is_outgoing);
 		conn->c_path[i].cp_index = i;
+		conn->c_path[i].cp_wq = rds_wq;
 	}
 	rcu_read_lock();
 	if (rds_destroy_pending(conn))
@@ -884,7 +885,7 @@ void rds_conn_path_drop(struct rds_conn_path *cp, bool destroy)
 		rcu_read_unlock();
 		return;
 	}
-	queue_work(rds_wq, &cp->cp_down_w);
+	queue_work(cp->cp_wq, &cp->cp_down_w);
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(rds_conn_path_drop);
@@ -909,7 +910,7 @@ void rds_conn_path_connect_if_down(struct rds_conn_path *cp)
 	}
 	if (rds_conn_path_state(cp) == RDS_CONN_DOWN &&
 	    !test_and_set_bit(RDS_RECONNECT_PENDING, &cp->cp_flags))
-		queue_delayed_work(rds_wq, &cp->cp_conn_w, 0);
+		queue_delayed_work(cp->cp_wq, &cp->cp_conn_w, 0);
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(rds_conn_path_connect_if_down);
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index 4248dfa816ebf..357128d34a546 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -457,7 +457,7 @@ void rds_ib_recv_refill(struct rds_connection *conn, int prefill, gfp_t gfp)
 	    (must_wake ||
 	    (can_wait && rds_ib_ring_low(&ic->i_recv_ring)) ||
 	    rds_ib_ring_empty(&ic->i_recv_ring))) {
-		queue_delayed_work(rds_wq, &conn->c_recv_w, 1);
+		queue_delayed_work(conn->c_path->cp_wq, &conn->c_recv_w, 1);
 	}
 	if (can_wait)
 		cond_resched();
diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
index 4190b90ff3b18..e35bbb6ffb689 100644
--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -419,7 +419,7 @@ void rds_ib_send_add_credits(struct rds_connection *conn, unsigned int credits)
 
 	atomic_add(IB_SET_SEND_CREDITS(credits), &ic->i_credits);
 	if (test_and_clear_bit(RDS_LL_SEND_FULL, &conn->c_flags))
-		queue_delayed_work(rds_wq, &conn->c_send_w, 0);
+		queue_delayed_work(conn->c_path->cp_wq, &conn->c_send_w, 0);
 
 	WARN_ON(IB_GET_SEND_CREDITS(credits) >= 16384);
 
diff --git a/net/rds/rds.h b/net/rds/rds.h
index a029e5fcdea72..b35afa2658cc4 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -118,6 +118,7 @@ struct rds_conn_path {
 
 	void			*cp_transport_data;
 
+	struct workqueue_struct	*cp_wq;
 	atomic_t		cp_state;
 	unsigned long		cp_send_gen;
 	unsigned long		cp_flags;
diff --git a/net/rds/send.c b/net/rds/send.c
index 0b3d0ef2f008b..3e3d028bc21ee 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -458,7 +458,8 @@ int rds_send_xmit(struct rds_conn_path *cp)
 			if (rds_destroy_pending(cp->cp_conn))
 				ret = -ENETUNREACH;
 			else
-				queue_delayed_work(rds_wq, &cp->cp_send_w, 1);
+				queue_delayed_work(cp->cp_wq,
+						   &cp->cp_send_w, 1);
 			rcu_read_unlock();
 		} else if (raced) {
 			rds_stats_inc(s_send_lock_queue_raced);
@@ -1380,7 +1381,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 		if (rds_destroy_pending(cpath->cp_conn))
 			ret = -ENETUNREACH;
 		else
-			queue_delayed_work(rds_wq, &cpath->cp_send_w, 1);
+			queue_delayed_work(cpath->cp_wq, &cpath->cp_send_w, 1);
 		rcu_read_unlock();
 	}
 	if (ret)
@@ -1470,10 +1471,10 @@ rds_send_probe(struct rds_conn_path *cp, __be16 sport,
 	rds_stats_inc(s_send_queued);
 	rds_stats_inc(s_send_pong);
 
-	/* schedule the send work on rds_wq */
+	/* schedule the send work on cp_wq */
 	rcu_read_lock();
 	if (!rds_destroy_pending(cp->cp_conn))
-		queue_delayed_work(rds_wq, &cp->cp_send_w, 1);
+		queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 1);
 	rcu_read_unlock();
 
 	rds_message_put(rm);
diff --git a/net/rds/tcp_recv.c b/net/rds/tcp_recv.c
index 7997a19d1da30..b7cf7f451430d 100644
--- a/net/rds/tcp_recv.c
+++ b/net/rds/tcp_recv.c
@@ -327,7 +327,7 @@ void rds_tcp_data_ready(struct sock *sk)
 	if (rds_tcp_read_sock(cp, GFP_ATOMIC) == -ENOMEM) {
 		rcu_read_lock();
 		if (!rds_destroy_pending(cp->cp_conn))
-			queue_delayed_work(rds_wq, &cp->cp_recv_w, 0);
+			queue_delayed_work(cp->cp_wq, &cp->cp_recv_w, 0);
 		rcu_read_unlock();
 	}
 out:
diff --git a/net/rds/tcp_send.c b/net/rds/tcp_send.c
index 7d284ac7e81a5..4e82c9644aa6a 100644
--- a/net/rds/tcp_send.c
+++ b/net/rds/tcp_send.c
@@ -201,7 +201,7 @@ void rds_tcp_write_space(struct sock *sk)
 	rcu_read_lock();
 	if ((refcount_read(&sk->sk_wmem_alloc) << 1) <= sk->sk_sndbuf &&
 	    !rds_destroy_pending(cp->cp_conn))
-		queue_delayed_work(rds_wq, &cp->cp_send_w, 0);
+		queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 0);
 	rcu_read_unlock();
 
 out:
diff --git a/net/rds/threads.c b/net/rds/threads.c
index 1f424cbfcbb47..639302bab51ef 100644
--- a/net/rds/threads.c
+++ b/net/rds/threads.c
@@ -89,8 +89,8 @@ void rds_connect_path_complete(struct rds_conn_path *cp, int curr)
 	set_bit(0, &cp->cp_conn->c_map_queued);
 	rcu_read_lock();
 	if (!rds_destroy_pending(cp->cp_conn)) {
-		queue_delayed_work(rds_wq, &cp->cp_send_w, 0);
-		queue_delayed_work(rds_wq, &cp->cp_recv_w, 0);
+		queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 0);
+		queue_delayed_work(cp->cp_wq, &cp->cp_recv_w, 0);
 	}
 	rcu_read_unlock();
 	cp->cp_conn->c_proposed_version = RDS_PROTOCOL_VERSION;
@@ -140,7 +140,7 @@ void rds_queue_reconnect(struct rds_conn_path *cp)
 		cp->cp_reconnect_jiffies = rds_sysctl_reconnect_min_jiffies;
 		rcu_read_lock();
 		if (!rds_destroy_pending(cp->cp_conn))
-			queue_delayed_work(rds_wq, &cp->cp_conn_w, 0);
+			queue_delayed_work(cp->cp_wq, &cp->cp_conn_w, 0);
 		rcu_read_unlock();
 		return;
 	}
@@ -151,7 +151,7 @@ void rds_queue_reconnect(struct rds_conn_path *cp)
 		 conn, &conn->c_laddr, &conn->c_faddr);
 	rcu_read_lock();
 	if (!rds_destroy_pending(cp->cp_conn))
-		queue_delayed_work(rds_wq, &cp->cp_conn_w,
+		queue_delayed_work(cp->cp_wq, &cp->cp_conn_w,
 				   rand % cp->cp_reconnect_jiffies);
 	rcu_read_unlock();
 
@@ -203,11 +203,11 @@ void rds_send_worker(struct work_struct *work)
 		switch (ret) {
 		case -EAGAIN:
 			rds_stats_inc(s_send_immediate_retry);
-			queue_delayed_work(rds_wq, &cp->cp_send_w, 0);
+			queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 0);
 			break;
 		case -ENOMEM:
 			rds_stats_inc(s_send_delayed_retry);
-			queue_delayed_work(rds_wq, &cp->cp_send_w, 2);
+			queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 2);
 			break;
 		default:
 			break;
@@ -228,11 +228,11 @@ void rds_recv_worker(struct work_struct *work)
 		switch (ret) {
 		case -EAGAIN:
 			rds_stats_inc(s_recv_immediate_retry);
-			queue_delayed_work(rds_wq, &cp->cp_recv_w, 0);
+			queue_delayed_work(cp->cp_wq, &cp->cp_recv_w, 0);
 			break;
 		case -ENOMEM:
 			rds_stats_inc(s_recv_delayed_retry);
-			queue_delayed_work(rds_wq, &cp->cp_recv_w, 2);
+			queue_delayed_work(cp->cp_wq, &cp->cp_recv_w, 2);
 			break;
 		default:
 			break;
-- 
2.43.0

[PATCH net-next v4 2/2] net/rds: Give each connection path its own workqueue

[Allison Henderson]

From: Allison Henderson <allison.henderson@oracle.com>

RDS was written to require ordered workqueues for "cp->cp_wq":
Work is executed in the order scheduled, one item at a time.

If these workqueues are shared across connections,
then work executed on behalf of one connection blocks work
scheduled for a different and unrelated connection.

Luckily we don't need to share these workqueues.
While it obviously makes sense to limit the number of
workers (processes) that ought to be allocated on a system,
a workqueue that doesn't have a rescue worker attached,
has a tiny footprint compared to the connection as a whole:
A workqueue costs ~900 bytes, including the workqueue_struct,
pool_workqueue, workqueue_attrs, wq_node_nr_active and the
node_nr_active flex array.  Each connection can have up to 8
(RDS_MPATH_WORKERS) paths for a worst case of ~7 KBytes per
connection.  While an RDS/IB connection totals only ~5 MBytes.

So we're getting a signficant performance gain
(90% of connections fail over under 3 seconds vs. 40%)
for a less than 0.02% overhead.

RDS doesn't even benefit from the additional rescue workers:
of all the reasons that RDS blocks workers, allocation under
memory pressue is the least of our concerns. And even if RDS
was stalling due to the memory-reclaim process, the work
executed by the rescue workers are highly unlikely to free up
any memory. If anything, they might try to allocate even more.

By giving each connection path its own workqueues, we allow
RDS to better utilize the unbound workers that the system
has available.

Signed-off-by: Somasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
 net/rds/connection.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index dc7323707f450..3743940423c83 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -269,7 +269,11 @@ static struct rds_connection *__rds_conn_create(struct net *net,
 		__rds_conn_path_init(conn, &conn->c_path[i],
 				     is_outgoing);
 		conn->c_path[i].cp_index = i;
-		conn->c_path[i].cp_wq = rds_wq;
+		conn->c_path[i].cp_wq =
+			alloc_ordered_workqueue("krds_cp_wq#%lu/%d", 0,
+						rds_conn_count, i);
+		if (!conn->c_path[i].cp_wq)
+			conn->c_path[i].cp_wq = rds_wq;
 	}
 	rcu_read_lock();
 	if (rds_destroy_pending(conn))
@@ -278,6 +282,9 @@ static struct rds_connection *__rds_conn_create(struct net *net,
 		ret = trans->conn_alloc(conn, GFP_ATOMIC);
 	if (ret) {
 		rcu_read_unlock();
+		for (i = 0; i < npaths; i++)
+			if (conn->c_path[i].cp_wq != rds_wq)
+				destroy_workqueue(conn->c_path[i].cp_wq);
 		kfree(conn->c_path);
 		kmem_cache_free(rds_conn_slab, conn);
 		conn = ERR_PTR(ret);
@@ -471,6 +478,11 @@ static void rds_conn_path_destroy(struct rds_conn_path *cp)
 	WARN_ON(work_pending(&cp->cp_down_w));
 
 	cp->cp_conn->c_trans->conn_free(cp->cp_transport_data);
+
+	if (cp->cp_wq != rds_wq) {
+		destroy_workqueue(cp->cp_wq);
+		cp->cp_wq = NULL;
+	}
 }
 
 /*
-- 
2.43.0

Re: [net-next,v4,1/2] net/rds: Add per cp work queue

[Jakub Kicinski]

This is an automated AI-generated review of your patch.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
commit 0d0b74852d11c82ac7d91c72b33c5dccb75934c5
Author: Allison Henderson <allison.henderson@oracle.com>

    net/rds: Add per cp work queue

This patch adds cp_wq to struct rds_conn_path and substitutes rds_wq with
cp->cp_wq throughout the RDS codebase to enable per-connection workqueue
management.

> diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
> index 4190b90ff3b1..e35bbb6ffb68 100644
> --- a/net/rds/ib_send.c
> +++ b/net/rds/ib_send.c
> @@ -419,7 +419,7 @@ void rds_ib_send_add_credits(struct rds_connection *conn, unsigned int credits)
>
>  	atomic_add(IB_SET_SEND_CREDITS(credits), &ic->i_credits);
>  	if (test_and_clear_bit(RDS_LL_SEND_FULL, &conn->c_flags))
> -		queue_delayed_work(rds_wq, &conn->c_send_w, 0);
> +		queue_delayed_work(conn->c_path->cp_wq, &conn->c_send_w, 0);
>
>  	WARN_ON(IB_GET_SEND_CREDITS(credits) >= 16384);

This substitution in rds_ib_send_add_credits() looks correct, but was the
similar call site in rds_ib_send_cqe_handler() in the same file
intentionally left unchanged? That function still uses:

    queue_delayed_work(rds_wq, &conn->c_send_w, 0);

Both functions have access to conn->c_path->cp_wq, so the inconsistency
seems unintentional. If some call sites should remain on the global rds_wq
while others use the per-connection workqueue, it would help to understand
the reasoning.

Similarly, rds_cong_queue_updates() in net/rds/cong.c still uses:

    queue_delayed_work(rds_wq, &cp->cp_send_w, 0);

even though it already has cp available. Should this also be updated to
use cp->cp_wq for consistency with the other substitutions?

Re: [net-next,v4,2/2] net/rds: Give each connection path its own workqueue

[Jakub Kicinski]

This is an automated AI-generated review of your patch.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
commit 95959b9817fcb113ceda9dcddc66514d2407af44
Author: Allison Henderson <allison.henderson@oracle.com>

net/rds: Give each connection path its own workqueue

This patch allocates per-connection-path workqueues instead of using
the shared rds_wq. This improves RDS failover performance by allowing
connections to process work independently.

> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index dc7323707f45..3743940423c8 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -269,7 +269,11 @@ static struct rds_connection *__rds_conn_create(struct net *net,
>  		__rds_conn_path_init(conn, &conn->c_path[i],
>  				     is_outgoing);
>  		conn->c_path[i].cp_index = i;
> -		conn->c_path[i].cp_wq = rds_wq;
> +		conn->c_path[i].cp_wq =
> +			alloc_ordered_workqueue("krds_cp_wq#%lu/%d", 0,
> +						rds_conn_count, i);
> +		if (!conn->c_path[i].cp_wq)
> +			conn->c_path[i].cp_wq = rds_wq;
>  	}
>  	rcu_read_lock();
>  	if (rds_destroy_pending(conn))
> @@ -278,6 +282,9 @@ static struct rds_connection *__rds_conn_create(struct net *net,
>  		ret = trans->conn_alloc(conn, GFP_ATOMIC);
>  	if (ret) {
>  		rcu_read_unlock();
> +		for (i = 0; i < npaths; i++)
> +			if (conn->c_path[i].cp_wq != rds_wq)
> +				destroy_workqueue(conn->c_path[i].cp_wq);
>  		kfree(conn->c_path);

The workqueues allocated above are properly destroyed here when
trans->conn_alloc() fails, but what happens in the "race to create"
paths later in this function?

When two threads race to create the same connection, one will win and
the other must discard its connection. Looking at the existing code
after this diff (around the "Creating passive conn" and "Creating
normal conn" comments), those cleanup paths call trans->conn_free(),
kfree(conn->c_path), and kmem_cache_free(), but they do not destroy
the per-path workqueues.

For example, in the "found" path for normal connections:

    if (found) {
        for (i = 0; i < npaths; i++) {
            cp = &conn->c_path[i];
            if (cp->cp_transport_data)
                trans->conn_free(cp->cp_transport_data);
        }
        kfree(conn->c_path);
        kmem_cache_free(rds_conn_slab, conn);
        conn = found;
    }

Could this leak the workqueues allocated earlier in the function?
Similar cleanup appears needed for the passive connection race path.

Note that destroy_workqueue() cannot be called while holding the
rds_conn_lock spinlock since it may sleep, so the fix would require
restructuring the cleanup to destroy workqueues before acquiring
the lock, similar to the trans->conn_alloc() failure path above.

[ ... ]

Re: [net-next,v4,1/2] net/rds: Add per cp work queue

[Allison Henderson]

On Thu, 2026-01-08 at 18:51 -0800, Jakub Kicinski wrote:
> This is an automated AI-generated review of your patch.
> 
> For local reproduction steps see https://urldefense.com/v3/__https://netdev-ai.bots.linux.dev/ai-local.html__;!!ACWV5N9M2RV99hQ!Jb4IwiQLlwSJFlqvRCi7Y54pEISEQKXyPiBtYXeM74mbOhBIwKUuPgo6RDWff-8JqJFtVL9t7X_PJiy6vw$ 
> 
> 
Hi Jakub,

Yes, I saw the AI response on patchwork yesterday and am working on an update already.  Thanks for the reproducer link,
I'll see if I can work out any more AI gripes.

Thank you!
Allison

> ---
> commit 0d0b74852d11c82ac7d91c72b33c5dccb75934c5
> Author: Allison Henderson <allison.henderson@oracle.com>
> 
>     net/rds: Add per cp work queue
> 
> This patch adds cp_wq to struct rds_conn_path and substitutes rds_wq with
> cp->cp_wq throughout the RDS codebase to enable per-connection workqueue
> management.
> 
> > diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
> > index 4190b90ff3b1..e35bbb6ffb68 100644
> > --- a/net/rds/ib_send.c
> > +++ b/net/rds/ib_send.c
> > @@ -419,7 +419,7 @@ void rds_ib_send_add_credits(struct rds_connection *conn, unsigned int credits)
> > 
> >  	atomic_add(IB_SET_SEND_CREDITS(credits), &ic->i_credits);
> >  	if (test_and_clear_bit(RDS_LL_SEND_FULL, &conn->c_flags))
> > -		queue_delayed_work(rds_wq, &conn->c_send_w, 0);
> > +		queue_delayed_work(conn->c_path->cp_wq, &conn->c_send_w, 0);
> > 
> >  	WARN_ON(IB_GET_SEND_CREDITS(credits) >= 16384);
> 
> This substitution in rds_ib_send_add_credits() looks correct, but was the
> similar call site in rds_ib_send_cqe_handler() in the same file
> intentionally left unchanged? That function still uses:
> 
>     queue_delayed_work(rds_wq, &conn->c_send_w, 0);
> 
> Both functions have access to conn->c_path->cp_wq, so the inconsistency
> seems unintentional. If some call sites should remain on the global rds_wq
> while others use the per-connection workqueue, it would help to understand
> the reasoning.
> 
> Similarly, rds_cong_queue_updates() in net/rds/cong.c still uses:
> 
>     queue_delayed_work(rds_wq, &cp->cp_send_w, 0);
> 
> even though it already has cp available. Should this also be updated to
> use cp->cp_wq for consistency with the other substitutions?