* [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap
2026-10-16 15:38 [PATCH net-next v7 00/13] net: mana: reconfigure by replacing the queue set Wei Hu
@ 2026-10-16 15:38 ` Wei Hu
[not found] ` <sashiko-outbox-175602@kernel.org>
0 siblings, 1 reply; 3+ messages in thread
From: Wei Hu @ 2026-10-16 15:38 UTC (permalink / raw)
To: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen
Cc: netdev, linux-rdma, linux-hyperv, linux-kernel, bpf, sdf, daniel,
hawk, ast, john.fastabend, weh, edumazet
From: Long Li <longli@microsoft.com>
Make the EQ pool port-owned so overlapping queue sets share EQs instead
of requiring old + new vector allocations. Allocate max_queues slots and
track populated entries with num_eqs.
Grow the pool before creating replacement CQs. Additional EQs survive
allocation failure in this patch and are released at port teardown.
Initialize EQ callbacks, context, owner phase and throttle before IRQ
publication, so a live shared IRQ cannot observe an uninitialized EQ.
Preserve the existing hardware-create and unwind ordering.
A shared EQ also outlives individual CQs. After stopping a CQ's NAPI and
work and tearing down its WQ object, flush its actual parent EQ while the
CQ callback remains published. Remove only that CQ's table entry and wait
for RCU IRQ readers before freeing the CQ and its callback owner.
Pair the IRQ lookup with release publication at all CQ publishers. For
Ethernet, publish only after NAPI and DIM initialization. The HWC and RDMA
sites need the matching stores, not changes to their teardown or recovery
policies.
The EQ markers and reader waits are control-path costs; the IRQ lookup
uses an acquire load without new validity checks. Report failed markers
using the existing error-logging convention. This does not add a reset or
reclamation policy for arbitrary failed hardware commands.
Release a newly registered DMA region if EQ construction fails before
the hardware queue takes ownership. Successful hardware creation
invalidates the software handle, so a later EQ-test failure does not
destroy a transferred region twice. This closes an existing unwind
gap required by repeated EQ-pool growth failures; retain the existing
hardware-command failure policy.
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Wei Hu <weh@microsoft.com>
---
drivers/infiniband/hw/mana/cq.c | 3 +-
.../net/ethernet/microsoft/mana/gdma_main.c | 26 ++--
.../net/ethernet/microsoft/mana/hw_channel.c | 3 +-
drivers/net/ethernet/microsoft/mana/mana_en.c | 115 ++++++++++++++----
include/net/mana/mana.h | 6 +-
5 files changed, 116 insertions(+), 37 deletions(-)
diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index d4e5e3f91268..0b6ad2547e0e 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -155,7 +155,8 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
gdma_cq->type = GDMA_CQ;
gdma_cq->cq.callback = mana_ib_cq_handler;
gdma_cq->id = cq->queue.id;
- gc->cq_table[cq->queue.id] = gdma_cq;
+ /* Pairs with the acquire load in mana_gd_process_eqe(). */
+ smp_store_release(&gc->cq_table[cq->queue.id], gdma_cq);
return 0;
}
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index ae0ed700b3b9..078b3666947a 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -768,7 +768,8 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs))
break;
- cq = gc->cq_table[cq_id];
+ /* Match release publication of the CQ and its callback state. */
+ cq = smp_load_acquire(&gc->cq_table[cq_id]);
if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
break;
@@ -1030,17 +1031,17 @@ static int mana_gd_create_eq(struct gdma_dev *gd,
return -EINVAL;
}
+ queue->eq.callback = spec->eq.callback;
+ queue->eq.context = spec->eq.context;
+ queue->head |= INITIALIZED_OWNER_BIT(log2_num_entries);
+ queue->eq.log2_throttle_limit = spec->eq.log2_throttle_limit ?: 1;
+
err = mana_gd_register_irq(queue, spec);
if (err) {
dev_err(dev, "Failed to register irq: %d\n", err);
return err;
}
- queue->eq.callback = spec->eq.callback;
- queue->eq.context = spec->eq.context;
- queue->head |= INITIALIZED_OWNER_BIT(log2_num_entries);
- queue->eq.log2_throttle_limit = spec->eq.log2_throttle_limit ?: 1;
-
if (create_hwq) {
err = mana_gd_create_hw_eq(gc, queue);
if (err)
@@ -1075,13 +1076,12 @@ static void mana_gd_destroy_cq(struct gdma_context *gc,
{
u32 id = queue->id;
- if (id >= gc->max_num_cqs)
- return;
-
- if (!gc->cq_table[id])
+ if (id >= gc->max_num_cqs || !gc->cq_table)
return;
- gc->cq_table[id] = NULL;
+ /* Leave a reused ID alone, but still drain readers of this CQ. */
+ cmpxchg(&gc->cq_table[id], queue, NULL);
+ synchronize_rcu();
}
int mana_gd_create_hwc_queue(struct gdma_dev *gd,
@@ -1273,8 +1273,10 @@ int mana_gd_create_mana_eq(struct gdma_dev *gd,
queue->gdma_dev = gd;
err = mana_gd_create_eq(gd, spec, true, queue);
- if (err)
+ if (err) {
+ mana_gd_destroy_dma_region(gc, gmi->dma_region_handle);
goto out;
+ }
*queue_ptr = queue;
return 0;
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 3bca4b683134..aefb8646cd79 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -702,7 +702,8 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
if (!gc->cq_table)
return -ENOMEM;
- gc->cq_table[cq->id] = cq;
+ /* Pairs with the acquire load in mana_gd_process_eqe(). */
+ smp_store_release(&gc->cq_table[cq->id], cq);
return 0;
}
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 7586972e78e0..4b2c8039b8fa 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1762,7 +1762,7 @@ void mana_destroy_eq(struct mana_port_context *apc)
debugfs_remove_recursive(apc->mana_eqs_debugfs);
apc->mana_eqs_debugfs = NULL;
- for (i = 0; i < apc->num_queues; i++) {
+ for (i = 0; i < apc->num_eqs; i++) {
eq = apc->eqs[i].eq;
if (!eq)
continue;
@@ -1774,6 +1774,7 @@ void mana_destroy_eq(struct mana_port_context *apc)
kfree(apc->eqs);
apc->eqs = NULL;
+ apc->num_eqs = 0;
}
EXPORT_SYMBOL_NS(mana_destroy_eq, "NET_MANA");
@@ -1802,9 +1803,11 @@ int mana_create_eq(struct mana_port_context *apc)
if (WARN_ON(apc->eqs))
return -EEXIST;
- apc->eqs = kzalloc_objs(struct mana_eq, apc->num_queues);
+ /* Keep EQ array addresses stable while CQs reference them. */
+ apc->eqs = kzalloc_objs(struct mana_eq, apc->max_queues);
if (!apc->eqs)
return -ENOMEM;
+ apc->num_eqs = 0;
spec.type = GDMA_EQ;
spec.monitor_avl_buf = false;
@@ -1834,6 +1837,7 @@ int mana_create_eq(struct mana_port_context *apc)
}
apc->eqs[i].eq->eq.irq = gic->irq;
mana_create_eq_debugfs(apc, i);
+ apc->num_eqs = i + 1;
}
return 0;
@@ -1843,6 +1847,61 @@ int mana_create_eq(struct mana_port_context *apc)
}
EXPORT_SYMBOL_NS(mana_create_eq, "NET_MANA");
+/* Grow the shared EQ pool without replacing live entries. */
+static int mana_grow_eqs(struct mana_port_context *apc, unsigned int need)
+{
+ struct gdma_dev *gd = apc->ac->gdma_dev;
+ struct gdma_context *gc = gd->gdma_context;
+ struct gdma_queue_spec spec = {};
+ struct gdma_irq_context *gic;
+ unsigned int i;
+ int err;
+ int msi;
+
+ if (WARN_ON(!apc->eqs))
+ return -EINVAL;
+
+ if (need > apc->max_queues)
+ return -EINVAL;
+
+ if (need <= apc->num_eqs)
+ return 0;
+
+ spec.type = GDMA_EQ;
+ spec.monitor_avl_buf = false;
+ spec.queue_size = EQ_SIZE;
+ spec.eq.callback = NULL;
+ spec.eq.context = apc->eqs;
+ spec.eq.log2_throttle_limit = LOG2_EQ_THROTTLE;
+
+ for (i = apc->num_eqs; i < need; i++) {
+ msi = (i + 1) % gc->num_msix_usable;
+
+ gic = mana_gd_get_gic(gc, !gc->msi_sharing, &msi);
+ if (IS_ERR(gic)) {
+ err = PTR_ERR(gic);
+ goto out;
+ }
+ spec.eq.msix_index = msi;
+
+ err = mana_gd_create_mana_eq(gd, &spec, &apc->eqs[i].eq);
+ if (err) {
+ dev_err(gc->dev, "Failed to grow EQ %u : %d\n", i, err);
+ mana_gd_put_gic(gc, !gc->msi_sharing, msi);
+ goto out;
+ }
+ apc->eqs[i].eq->eq.irq = gic->irq;
+ mana_create_eq_debugfs(apc, i);
+ apc->num_eqs = i + 1;
+ }
+
+ return 0;
+out:
+ /* Retain partial growth for reuse; the live set still needs this pool.
+ */
+ return err;
+}
+
static int mana_fence_rq(struct mana_port_context *apc, struct mana_rxq *rxq)
{
struct mana_fence_rq_resp resp = {};
@@ -2653,12 +2712,25 @@ static void mana_schedule_napi(void *context, struct gdma_queue *gdma_queue)
static void mana_deinit_cq(struct mana_port_context *apc, struct mana_cq *cq)
{
- struct gdma_dev *gd = apc->ac->gdma_dev;
+ struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
+ struct gdma_queue *gdma_cq = cq->gdma_cq;
+ struct gdma_queue *eq;
+ int err;
- if (!cq->gdma_cq)
+ if (!gdma_cq)
return;
- mana_gd_destroy_queue(gd->gdma_context, cq->gdma_cq);
+ eq = gdma_cq->cq.parent;
+ if (gdma_cq->id < gc->max_num_cqs && eq &&
+ eq->id != INVALID_QUEUE_ID) {
+ /* Flush queued events after WQ teardown, before removing this CQ. */
+ err = mana_gd_test_eq(gc, eq);
+ if (err && mana_en_need_log(apc, err))
+ netdev_err(apc->ndev, "Failed to flush EQ %u for CQ %u: %d\n",
+ eq->id, gdma_cq->id, err);
+ }
+
+ mana_gd_destroy_queue(gc, gdma_cq);
}
static void mana_deinit_txq(struct mana_port_context *apc, struct mana_txq *txq)
@@ -2853,8 +2925,6 @@ static int mana_create_txq(struct mana_port_context *apc,
goto out;
}
- gc->cq_table[cq->gdma_id] = cq->gdma_cq;
-
mana_create_txq_debugfs(apc, i);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cq->napi.state);
@@ -2869,6 +2939,9 @@ static int mana_create_txq(struct mana_port_context *apc,
napi_enable_locked(&cq->napi);
txq->napi_initialized = true;
+ /* Publish the initialized NAPI/DIM state to the EQ handler. */
+ smp_store_release(&gc->cq_table[cq->gdma_id], cq->gdma_cq);
+
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
}
@@ -3179,8 +3252,6 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
goto out;
}
- gc->cq_table[cq->gdma_id] = cq->gdma_cq;
-
netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
WARN_ON(xdp_rxq_info_reg(&rxq->xdp_rxq, ndev, rxq_idx,
@@ -3196,6 +3267,9 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
napi_enable_locked(&cq->napi);
+ /* Publish the initialized NAPI/DIM state to the EQ handler. */
+ smp_store_release(&gc->cq_table[cq->gdma_id], cq->gdma_cq);
+
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
out:
if (!err)
@@ -3799,7 +3873,6 @@ static int mana_dealloc_queues(struct net_device *ndev)
static void mana_qset_snapshot(const struct mana_port_context *ctx,
struct mana_qset *out)
{
- out->eqs = ctx->eqs;
out->tx_qp = ctx->tx_qp;
out->rxqs = ctx->rxqs;
out->indir_table = ctx->indir_table;
@@ -3816,7 +3889,6 @@ static void mana_qset_snapshot(const struct mana_port_context *ctx,
static void mana_qset_install(struct mana_port_context *ctx,
const struct mana_qset *qset)
{
- ctx->eqs = qset->eqs;
ctx->tx_qp = qset->tx_qp;
ctx->rxqs = qset->rxqs;
ctx->indir_table = qset->indir_table;
@@ -3829,7 +3901,9 @@ static void mana_qset_install(struct mana_port_context *ctx,
ctx->priv_flags = qset->priv_flags;
}
-/* Copy the vport identity without borrowing the live queues. */
+/* Scratch starts without SQs/RQs and borrows the port's EQ pool. Never call
+ * mana_destroy_eq() on it.
+ */
struct mana_port_context *mana_qset_scratch_alloc(struct mana_port_context *apc)
{
struct mana_port_context *scratch;
@@ -3840,13 +3914,11 @@ struct mana_port_context *mana_qset_scratch_alloc(struct mana_port_context *apc)
*scratch = *apc;
- scratch->eqs = NULL;
scratch->tx_qp = NULL;
scratch->rxqs = NULL;
scratch->indir_table = NULL;
scratch->rxobj_table = NULL;
scratch->default_rxobj = INVALID_MANA_HANDLE;
- scratch->mana_eqs_debugfs = NULL;
/* Do not consume the live set's pre-allocated RX buffers. */
scratch->rxbufs_pre = NULL;
@@ -3864,7 +3936,8 @@ void mana_qset_scratch_free(struct mana_port_context *scratch)
kvfree(scratch);
}
-int mana_alloc_qset(struct mana_port_context *scratch, unsigned int num_queues,
+int mana_alloc_qset(struct mana_port_context *apc,
+ struct mana_port_context *scratch, unsigned int num_queues,
unsigned int rx_queue_size, unsigned int tx_queue_size,
u32 priv_flags, struct mana_qset *out)
{
@@ -3886,13 +3959,16 @@ int mana_alloc_qset(struct mana_port_context *scratch, unsigned int num_queues,
if (err)
goto cleanup_rxq_array;
- err = mana_create_eq(scratch);
+ err = mana_grow_eqs(apc, num_queues);
if (err)
goto cleanup_rss;
+ scratch->eqs = apc->eqs;
+ scratch->num_eqs = apc->num_eqs;
+
err = mana_create_txq(scratch, ndev);
if (err)
- goto cleanup_eq;
+ goto cleanup_rss;
err = mana_add_rx_queues(scratch, ndev);
if (err)
@@ -3906,8 +3982,6 @@ int mana_alloc_qset(struct mana_port_context *scratch, unsigned int num_queues,
cleanup_rxq:
mana_destroy_rxqs(scratch);
mana_destroy_txq(scratch);
-cleanup_eq:
- mana_destroy_eq(scratch);
cleanup_rss:
mana_cleanup_indir_table(scratch);
cleanup_rxq_array:
@@ -3927,7 +4001,7 @@ void mana_free_qset(struct mana_port_context *scratch, struct mana_qset *qset)
ASSERT_RTNL();
- if (!qset->rxqs && !qset->tx_qp && !qset->eqs)
+ if (!qset->rxqs && !qset->tx_qp)
return;
if (qset->tx_qp) {
@@ -3961,7 +4035,6 @@ void mana_free_qset(struct mana_port_context *scratch, struct mana_qset *qset)
mana_chn_xdp_release(retiring_prog, retiring_queues);
mana_destroy_txq(scratch);
- mana_destroy_eq(scratch);
mana_cleanup_indir_table(scratch);
kfree(scratch->rxqs);
scratch->rxqs = NULL;
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 1d91b62299f5..12a65145cd3f 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -574,7 +574,9 @@ struct mana_port_context {
u8 mac_addr[ETH_ALEN];
+ /* Port-owned EQ pool: max_queues slots, num_eqs populated. */
struct mana_eq *eqs;
+ unsigned int num_eqs;
struct dentry *mana_eqs_debugfs;
enum TRI_STATE rss_state;
@@ -677,7 +679,6 @@ struct mana_port_context {
};
struct mana_qset {
- struct mana_eq *eqs;
struct mana_tx_qp **tx_qp;
struct mana_rxq **rxqs;
@@ -705,7 +706,8 @@ int mana_detach(struct net_device *ndev, bool from_close);
struct mana_port_context *
mana_qset_scratch_alloc(struct mana_port_context *apc);
void mana_qset_scratch_free(struct mana_port_context *scratch);
-int mana_alloc_qset(struct mana_port_context *scratch, unsigned int num_queues,
+int mana_alloc_qset(struct mana_port_context *apc,
+ struct mana_port_context *scratch, unsigned int num_queues,
unsigned int rx_queue_size, unsigned int tx_queue_size,
u32 priv_flags, struct mana_qset *out);
void mana_free_qset(struct mana_port_context *scratch, struct mana_qset *qset);
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap
[not found] ` <sashiko-outbox-175602@kernel.org>
@ 2026-10-20 4:30 ` Wei Hu
0 siblings, 0 replies; 3+ messages in thread
From: Wei Hu @ 2026-10-20 4:30 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-rdma, linux-hyperv, bpf, netdev, weh, kuba
Hi,
> Should mana_ib_remove_cq_cb() use cmpxchg() and synchronize_rcu() for
> teardown, similar to what was added for Ethernet CQs in
> mana_gd_destroy_cq()?
I will leave the RDMA teardown path unchanged in v8. The callback-wrapper
lifetime issue is real, but mana_ib_remove_cq_cb() and mana_ib_destroy_cq()
already have this ordering in both unmodified upstream bases used for v6
and v7. The acquire/release publication change does not introduce that
reader/free race or resolve it. An independent RDMA lifetime fix should
cover the hardware CQ and pending events as well as the software readers;
I am not including that separate change in this queue-set revision.
> Can gc->cq_table be NULL when this acquire load executes?
I will also leave this initialization path unchanged in v8. There is a
window after max_num_cqs is populated and before cq_table is allocated.
A spurious or premature completion in that window could hit the missing
table-root check. That window and the unguarded lookup already exist in
both upstream bases, before this series.
For normal HWC startup, WQEs are posted only after channel establishment
returns and the CQ table is installed. I have not identified a normal
completion producer in the window described here. This is a conditional
hardware-protocol hardening issue, not a demonstrated new startup failure
caused by the publication change.
> Could this sequential mana_gd_test_eq() call lead to an unbounded RTNL
> stall if an EQ stops delivering interrupts?
I will address the additional per-CQ timeout amplification in v8. These
extra waits are introduced by the queue-set series and are already in v6.
The correction will stop the retiring WQs, keep their CQ callbacks alive,
and issue all required parent-EQ markers before waiting. Per-EQ
completions will allow one shared 30-second completion budget per RX or TX
cleanup batch. CQ withdrawal and the existing RCU reader wait remain
after the batch, including the partial-allocation cleanup paths.
One detail of the reported bound needs qualification: each TX/RX pair has
its own parent EQ. One stalled parent normally adds two CQ waits, about
60 seconds. The 128 * 30-second case requires all 64 parents to stall
while the hardware channel remains responsive. The cumulative delay is
finite, but it is still a real problem introduced by these extra waits.
The correction bounds the added CQ-marker completion waits, not all RTNL
hold time. Existing per-EQ destruction waits and hardware-channel request
time remain separate. It also preserves the existing best-effort cleanup
policy after a failed marker, rather than claiming new hardware recovery
guarantees.
Thanks,
Wei
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap
@ 2026-10-20 16:46 netdev-bot+sashiko
0 siblings, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-10-20 16:46 UTC (permalink / raw)
To: weh
Cc: longli, kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, wei.liu, decui, shradhagupta, horms, ernis,
stephen, netdev, linux-rdma, linux-hyperv, linux-kernel, bpf, sdf,
daniel, hawk, ast, john.fastabend, weh, edumazet
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 0 (1) · Medium: 1 (1) · Low: 2
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] Every Ethernet CQ teardown now adds a blocking
mana_gd_test_eq() on its parent EQ (mana_deinit_cq()) and an…
- [Low] This feature patch ('share the EQ pool across a queue-set swap')
bundles two fixes for bugs that can be hit today, with no Fixes: tag,…
- [Low] A comment in struct mana_port_context (include/net/mana/mana.h)
is out of date.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch's
new reader protocol does not cover it, and the commit message says it…
- [Medium] This problem wasn't introduced by this patch, but the HWC
teardown frees CQ callback state before IRQ readers are drained.
--
net: mana: share the EQ pool across a queue-set swap
This commit makes the MANA EQ pool port-owned. The old and new queue sets
in a swap now share EQs, and mana_grow_eqs() grows the pool when needed.
It also moves cq_table publication to release/acquire and adds a per-CQ
EQ flush plus an RCU grace period when a CQ is torn down. EQ
initialization now happens before IRQ registration, and the DMA region is
released when EQ creation fails.
> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> index d4e5e3f912689..0b6ad2547e0ef 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
> @@ -155,7 +155,8 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
> gdma_cq->type = GDMA_CQ;
> gdma_cq->cq.callback = mana_ib_cq_handler;
> gdma_cq->id = cq->queue.id;
> - gc->cq_table[cq->queue.id] = gdma_cq;
> + /* Pairs with the acquire load in mana_gd_process_eqe(). */
> + smp_store_release(&gc->cq_table[cq->queue.id], gdma_cq);
> return 0;
> }
[Severity: High]
This is a pre-existing issue, not one introduced by this patch. This patch
adds a release/acquire and RCU reader protocol for cq_table. Shouldn't the
matching RDMA remover change as well?
mana_ib_remove_cq_cb() still frees the published gdma_queue before it
clears the slot, and it never waits for IRQ readers:
drivers/infiniband/hw/mana/cq.c:mana_ib_remove_cq_cb() {
...
kfree(gc->cq_table[cq->queue.id]);
gc->cq_table[cq->queue.id] = NULL;
}
mana_ib_destroy_cq() calls it before the hardware CQ is destroyed:
mana_ib_remove_cq_cb(mdev, cq);
...
mana_ib_gd_destroy_cq(mdev, cq);
So the CQ can still raise EQEs at that point. Raw/RSS QPs attach these CQs
to the port's mpc->eqs[] pool, which this patch makes long-lived.
At the same time, mana_gd_process_eqe() runs under rcu_read_lock() in
mana_gd_intr() and does:
cq = smp_load_acquire(&gc->cq_table[cq_id]);
if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
break;
if (cq->cq.callback)
cq->cq.callback(cq->cq.context, cq);
Suppose another CPU has already loaded the pointer when kfree() runs. Could
it read freed memory and call through a function pointer taken from it?
After destroy returns, the IB core also frees the mana_ib_cq that
mana_ib_cq_handler() dereferences.
The other CQ removers now unpublish, call synchronize_rcu(), and then
free, as mana_gd_destroy_cq() does. This is the last plain cq_table store
left, yet the commit message says:
The HWC and RDMA sites need the matching stores, not changes to
their teardown or recovery policies.
Does that hold for mana_ib_remove_cq_cb()? Userspace can reach it through
ibv_destroy_cq() while completions are still arriving on the same EQ. The
code still looks unchanged at the end of the series.
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index ae0ed700b3b93..078b3666947a4 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
[ ... ]
> @@ -1030,17 +1031,17 @@ static int mana_gd_create_eq(struct gdma_dev *gd,
> return -EINVAL;
> }
>
> + queue->eq.callback = spec->eq.callback;
> + queue->eq.context = spec->eq.context;
> + queue->head |= INITIALIZED_OWNER_BIT(log2_num_entries);
> + queue->eq.log2_throttle_limit = spec->eq.log2_throttle_limit ?: 1;
> +
> err = mana_gd_register_irq(queue, spec);
> if (err) {
> dev_err(dev, "Failed to register irq: %d\n", err);
> return err;
> }
>
> - queue->eq.callback = spec->eq.callback;
> - queue->eq.context = spec->eq.context;
> - queue->head |= INITIALIZED_OWNER_BIT(log2_num_entries);
> - queue->eq.log2_throttle_limit = spec->eq.log2_throttle_limit ?: 1;
> -
> if (create_hwq) {
> err = mana_gd_create_hw_eq(gc, queue);
> if (err)
[ ... ]
> @@ -1273,8 +1273,10 @@ int mana_gd_create_mana_eq(struct gdma_dev *gd,
> queue->gdma_dev = gd;
>
> err = mana_gd_create_eq(gd, spec, true, queue);
> - if (err)
> + if (err) {
> + mana_gd_destroy_dma_region(gc, gmi->dma_region_handle);
> goto out;
> + }
>
> *queue_ptr = queue;
> return 0;
[Severity: Low]
This isn't a bug, but both of these changes look like fixes for problems
that exist today, separate from the queue-set swap. The commit message
says the reorder ensures "a live shared IRQ cannot observe an
uninitialized EQ". It also says the DMA region release "closes an
existing unwind gap".
Before this patch, mana_gd_create_eq() linked the queue into gic->eq_list
with list_add_rcu() in mana_gd_register_irq(). That happened before
eq.callback, eq.context, the owner bit and log2_throttle_limit were set.
With msi_sharing, mana_gd_intr() could walk the new EQ while its head and
owner phase were still zero.
Also, when mana_gd_create_eq() failed, mana_gd_create_mana_eq() only
called mana_gd_free_memory(). That leaked the DMA region registered by
mana_gd_create_dma_region().
Both paths are reachable without any queue-set swap, through
mana_alloc_queues()->mana_create_eq() and through RDMA's
mana_ib_create_eqs().
Would it make sense to split these into separate patches with Fixes: tags,
so stable can pick them up?
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 3bca4b683134b..aefb8646cd79c 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -702,7 +702,8 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
> if (!gc->cq_table)
> return -ENOMEM;
>
> - gc->cq_table[cq->id] = cq;
> + /* Pairs with the acquire load in mana_gd_process_eqe(). */
> + smp_store_release(&gc->cq_table[cq->id], cq);
>
> return 0;
> }
[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch, but the
HWC teardown path doesn't get the new grace period either.
mana_hwc_destroy_channel() does:
if (gc->max_num_cqs > 0) {
mana_smc_teardown_hwc(&gc->shm_channel, false);
gc->max_num_cqs = 0;
}
Because max_num_cqs is now 0, mana_gd_destroy_cq() returns early on its
id >= gc->max_num_cqs check and skips synchronize_rcu(). Then
mana_hwc_destroy_cq() does:
kfree(hwc_cq->comp_buf);
if (hwc_cq->gdma_cq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
if (hwc_cq->gdma_eq)
mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
The EQ is deregistered and RCU-synchronized only after comp_buf and the
gdma_cq have already been freed.
An HWC EQ handler could still be running mana_hwc_comp_event() after
mana_smc_teardown_hwc() returns. That can happen with threaded or
preempted IRQ handling, or with a late HWC response during removal,
suspend or reset. Can that handler then touch the freed comp_buf through:
completions = hwc_cq->comp_buf;
or touch the freed gdma_cq? The commit message deliberately leaves the
HWC teardown policy alone, so this only points out the remaining gap.
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 7586972e78e09..4b2c8039b8fa3 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -2653,12 +2712,25 @@ static void mana_schedule_napi(void *context, struct gdma_queue *gdma_queue)
>
> static void mana_deinit_cq(struct mana_port_context *apc, struct mana_cq *cq)
> {
> - struct gdma_dev *gd = apc->ac->gdma_dev;
> + struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
> + struct gdma_queue *gdma_cq = cq->gdma_cq;
> + struct gdma_queue *eq;
> + int err;
>
> - if (!cq->gdma_cq)
> + if (!gdma_cq)
> return;
>
> - mana_gd_destroy_queue(gd->gdma_context, cq->gdma_cq);
> + eq = gdma_cq->cq.parent;
> + if (gdma_cq->id < gc->max_num_cqs && eq &&
> + eq->id != INVALID_QUEUE_ID) {
> + /* Flush queued events after WQ teardown, before removing this CQ. */
> + err = mana_gd_test_eq(gc, eq);
> + if (err && mana_en_need_log(apc, err))
> + netdev_err(apc->ndev, "Failed to flush EQ %u for CQ %u: %d\n",
> + eq->id, gdma_cq->id, err);
> + }
> +
> + mana_gd_destroy_queue(gc, gdma_cq);
> }
[Severity: Medium]
How much slower does teardown get with this? Every Ethernet CQ now does a
blocking mana_gd_test_eq() on its parent EQ here. It also does an
unconditional synchronize_rcu() in mana_gd_destroy_cq():
cmpxchg(&gc->cq_table[id], queue, NULL);
synchronize_rcu();
Both run one CQ at a time from mana_destroy_txq() and mana_destroy_rxq().
They hold RTNL and the netdev instance lock, and they run on close,
detach, reset, create unwind and queue-set free.
mana_gd_test_eq() takes gc->eq_test_event_mutex, sends
GDMA_GENERATE_TEST_EQE over the HWC, and waits up to 30 * HZ. TX CQ i and
RX CQ i share EQ i. After that, mana_destroy_eq()->mana_gd_destroy_eq()
flushes each EQ again and adds another grace period in
mana_gd_deregister_irq().
So teardown seems to go from N test-EQ round trips and N grace periods to
3N of each. synchronize_rcu() now also runs for CQs that were never
published; the old code returned early when the slot was empty.
A failed flush is only logged. Suppose an EQ stops delivering test events
while the HWC still answers. Wouldn't each CQ on that EQ wait the full 30
seconds, and then mana_gd_destroy_eq() wait again?
Would one flush per distinct EQ, plus one grace period after all slots
are cleared, be enough here? This still seems to be the case at the end
of the series.
[ ... ]
> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 1d91b62299f56..12a65145cd3fd 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -574,7 +574,9 @@ struct mana_port_context {
>
> u8 mac_addr[ETH_ALEN];
>
> + /* Port-owned EQ pool: max_queues slots, num_eqs populated. */
> struct mana_eq *eqs;
> + unsigned int num_eqs;
> struct dentry *mana_eqs_debugfs;
>
> enum TRI_STATE rss_state;
[Severity: Low]
This isn't a bug, but an older comment further down in struct
mana_port_context still says:
/* Create num_queues EQs, SQs, SQ-CQs, RQs and RQ-CQs, respectively. */
unsigned int max_queues;
unsigned int num_queues;
mana_destroy_eq() now iterates over num_eqs, which can exceed num_queues
after mana_grow_eqs(). Should that comment be updated as well? It is
still there at the end of the series.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791710735.git.weh%40linux.microsoft.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-20 16:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-20 16:46 [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap netdev-bot+sashiko
-- strict thread matches above, loose matches on Subject: below --
2026-10-16 15:38 [PATCH net-next v7 00/13] net: mana: reconfigure by replacing the queue set Wei Hu
2026-10-16 15:38 ` [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap Wei Hu
[not found] ` <sashiko-outbox-175602@kernel.org>
2026-10-20 4:30 ` Wei Hu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox