Skip to content

Commit 891ba94

Browse files
allisonhendersonNipaLocal
authored andcommitted
net/rds: Give each connection path its own workqueue
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> Signed-off-by: NipaLocal <nipa@local>
1 parent 46f0df8 commit 891ba94

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

net/rds/connection.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,11 @@ static struct rds_connection *__rds_conn_create(struct net *net,
269269
__rds_conn_path_init(conn, &conn->c_path[i],
270270
is_outgoing);
271271
conn->c_path[i].cp_index = i;
272-
conn->c_path[i].cp_wq = rds_wq;
272+
conn->c_path[i].cp_wq =
273+
alloc_ordered_workqueue("krds_cp_wq#%lu/%d", 0,
274+
rds_conn_count, i);
275+
if (!conn->c_path[i].cp_wq)
276+
conn->c_path[i].cp_wq = rds_wq;
273277
}
274278
rcu_read_lock();
275279
if (rds_destroy_pending(conn))
@@ -278,6 +282,9 @@ static struct rds_connection *__rds_conn_create(struct net *net,
278282
ret = trans->conn_alloc(conn, GFP_ATOMIC);
279283
if (ret) {
280284
rcu_read_unlock();
285+
for (i = 0; i < npaths; i++)
286+
if (conn->c_path[i].cp_wq != rds_wq)
287+
destroy_workqueue(conn->c_path[i].cp_wq);
281288
kfree(conn->c_path);
282289
kmem_cache_free(rds_conn_slab, conn);
283290
conn = ERR_PTR(ret);
@@ -471,6 +478,11 @@ static void rds_conn_path_destroy(struct rds_conn_path *cp)
471478
WARN_ON(work_pending(&cp->cp_down_w));
472479

473480
cp->cp_conn->c_trans->conn_free(cp->cp_transport_data);
481+
482+
if (cp->cp_wq != rds_wq) {
483+
destroy_workqueue(cp->cp_wq);
484+
cp->cp_wq = NULL;
485+
}
474486
}
475487

476488
/*

0 commit comments

Comments
 (0)