Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: mana: duplicate mana_hwc_teardown_queues() on HWC
rebuild failure
--
commit 63009cdb17b7f248623a5c752d26416fb4226096
Author: Long Li [off-list ref]
net: mana: add dynamic HWC queue depth with reinit path
This commit adds dynamic HWC queue depth negotiation by rebuilding the
bootstrap hardware channel when the PF reports a supported queue depth
between 2 and 128, with fallback to fresh bootstrap queues if the rebuild
fails.
Link: https://lore.kernel.org/r/20260908035201.402424-5-longli@microsoft.com (local)
Link: https://lore.kernel.org/r/179196575846.434549.7933818740976196422@kernel.org (local)
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 8bc00001d20e5..867f8dd3ea547 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1088,16 +1176,11 @@ static void mana_hwc_fence_channel(struct gdma_context *gc,
mana_hwc_unpublish_cq(gc, hwc->cq->gdma_cq);
}
-static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
+static int mana_hwc_teardown_queues(struct hw_channel_context *hwc)
{
struct gdma_context *gc = hwc->gdma_dev->gdma_context;
int err;
- mana_hwc_stop_channel(hwc);
-
- /* An unacknowledged destroy leaves the PF's mappings live. Fence
- * software dispatch, but retain every PF-visible allocation.
- */
err = mana_smc_teardown_hwc(&gc->shm_channel, false,
&hwc->setup_active);
if (err) {@@ -1105,28 +1188,26 @@ static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
"HWC teardown failed: %d, retaining PF-visible resources\n",
err);
mana_hwc_fence_channel(gc, hwc);
- mana_hwc_retain_channel(hwc);
- return false;
+ return err;
}
- mana_hwc_clear_cq_table(gc);
-
- /* Fence the EQ before releasing the remaining HWC state. */
- if (hwc->cq)
- mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
-
- if (hwc->txq)
- mana_hwc_destroy_wq(hwc, hwc->txq);
+ mana_hwc_destroy_queues(hwc);
- if (hwc->rxq)
- mana_hwc_destroy_wq(hwc, hwc->rxq);
+ return 0;
+}
- kfree(hwc->caller_ctx);
- hwc->caller_ctx = NULL;
+static bool mana_hwc_release_channel(struct hw_channel_context *hwc)
+{
+ struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+ int err;
- mana_gd_free_res_map(&hwc->inflight_msg_res);
+ mana_hwc_stop_channel(hwc);
- hwc->num_inflight_msg = 0;
+ err = mana_hwc_teardown_queues(hwc);
+ if (err) {
+ mana_hwc_retain_channel(hwc);
+ return false;
+ }
[ ... ]
quoted hunk ↗ jump to hunk
+static int mana_hwc_rebuild_channel(struct hw_channel_context *hwc,
+ u16 q_depth,
+ struct mana_hwc_init_report *report)
+{
+ u32 timeout_ms = mana_hwc_timeout_read(hwc);
+ int cleanup_err;
+ int err;
+
+ err = mana_hwc_teardown_queues(hwc);
+ if (err)
+ return err;
+
+ err = mana_hwc_build_channel(hwc, q_depth, report);
+ if (!err)
+ err = mana_hwc_validate_report(hwc, report, q_depth,
+ true, true);
+ if (!err)
+ err = mana_hwc_finish_channel(hwc, report);
+ if (!err)
+ return 0;
+
+ dev_warn(hwc->dev,
+ "HWC depth %u rebuild failed, restoring bootstrap: %d\n",
+ q_depth, err);
+
+ cleanup_err = mana_hwc_teardown_queues(hwc);
+ if (cleanup_err)
+ return cleanup_err;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1234,7 +1452,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
gd->pdid = INVALID_PDID;
gd->doorbell = INVALID_DOORBELL;
- err = mana_hwc_create_bootstrap_channel(hwc);
+ err = mana_hwc_prepare_channel(hwc);
if (err) {
mana_hwc_release_channel(hwc);
return err;
[Severity: Medium]
Does mana_hwc_release_channel() issue a duplicate mana_hwc_teardown_queues()
when mana_hwc_rebuild_channel() fails inside mana_hwc_teardown_queues()?
mana_smc_teardown_hwc_locked() only clears hwc->setup_active when the SMC
teardown transaction succeeds. If mana_hwc_teardown_queues() fails in
mana_hwc_rebuild_channel(), it fences the channel and returns the error with
hwc->setup_active still true, and the error path in mana_hwc_create_channel()
then calls mana_hwc_release_channel() -> mana_hwc_teardown_queues() a second
time on the same channel. On an unresponsive shared-memory channel this
stalls for a second 20-40s poll timeout; if the PF completes the first
SMC_MSG_TYPE_DESTROY_HWC right after the first read times out and rejects the
second destroy with a status error, mana_hwc_release_channel() retains the
already-destroyed channel.
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794208055.git.weh@linux.microsoft.com?part=4