net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In mana_rdma_probe() (drivers/net/ethernet/microsoft/mana/mana_en.c:4296) the servicing.

--

net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe()

mana_rdma_remove() sets gd->rdma_teardown and nothing ever clears it, so after the first hardware service reset every GDMA_EQE_HWC_SOC_SERVICE event returns early. This patch clears gd->is_suspended and re-opens the gate with smp_store_release() in mana_rdma_probe(), paired with a new smp_load_acquire() in mana_rdma_service_handle().

drivers/net/ethernet/microsoft/mana/mana_en.c static void mana_rdma_service_handle(struct

Quoted 6 lines

drivers/net/ethernet/microsoft/mana/mana_en.c +0 -0

index 7a1ac853e3ab..d8e847df7a44 100644
@@ -3983,7 +3983,8 @@ static void mana_rdma_service_handle(struct

work_struct *work)

Quoted 9 lines

drivers/net/ethernet/microsoft/mana/mana_en.c +2 -1

 	struct device *dev = gd->gdma_context->dev;
 	int ret;
 
-	if (READ_ONCE(gd->rdma_teardown))
+	/* Pairs with the smp_store_release() in mana_rdma_probe(). */
+	if (smp_load_acquire(&gd->rdma_teardown))
 		goto out;
 
 	switch (serv_work->event) {

[ ... ]

drivers/net/ethernet/microsoft/mana/mana_en.c int mana_rdma_probe(struct gdma_dev *gd) · line 4283

Quoted 5 lines

drivers/net/ethernet/microsoft/mana/mana_en.c +1 -0

@@ -4279,6 +4280,21 @@ int mana_rdma_probe(struct gdma_dev *gd)
 	if (err)
 		return err;
 
+	/* Clear the state left by a previous mana_rdma_remove() so

servicing

drivers/net/ethernet/microsoft/mana/mana_en.c int mana_rdma_probe(struct gdma_dev *gd)

Quoted 7 lines

drivers/net/ethernet/microsoft/mana/mana_en.c +7 -0

+	 * events are handled again after a reset cycle.
+	 */
+	gd->is_suspended = false;
+
+	/* Publish is_suspended before re-opening the gate, so the handler
+	 * cannot observe an open gate with a stale is_suspended.  Pairs
+	 * with the smp_load_acquire() in mana_rdma_service_handle().

This

drivers/net/ethernet/microsoft/mana/mana_en.c int mana_rdma_probe(struct gdma_dev *gd) · line 4300

Quoted 10 lines

drivers/net/ethernet/microsoft/mana/mana_en.c +7 -0

+	 * matters on the reset path, where mana_rdma_remove() closed the
+	 * gate and drained the workqueue; on the initial probe path the
+	 * gate was never closed and both flags are already clear.  It does
+	 * not order gd->adev, which add_adev() publishes below.
+	 */
+	smp_store_release(&gd->rdma_teardown, false);
+
 	err = add_adev(gd, "rdma");
 	if (err)
 		mana_gd_deregister_device(gd);

[Severity: Medium]

Would it be better to open the gate after add_adev() succeeds rather than before it?

As written, the store that re-enables the handler happens while gd->adev is still NULL from the remove half of the reset cycle. The publication happens later, at the end of add_adev():

drivers/net/ethernet/microsoft/mana/mana_en.c:add_adev() {
	...
	ret = auxiliary_device_add(adev);
	if (ret)
		goto add_fail;

	gd->adev = adev;
	...
}

A GDMA_SERVICE_TYPE_RDMA_SUSPEND handled in that window passes the gate and then hits the gd->adev test in mana_rdma_service_handle():

	case GDMA_SERVICE_TYPE_RDMA_SUSPEND:
		if (!gd->adev || gd->is_suspended)
			break;

		remove_adev(gd);
		gd->is_suspended = true;
		break;

so the suspend request is discarded, probe finishes with a live aux device and is_suspended == false, and the matching RESUME is then also swallowed by the "if (!gd->is_suspended) break;" test. Is the resulting state (host asked for a quiesce, RDMA aux device still live) recoverable without another reset?

Is the concurrency here real on the reset path? From what I can follow, the probe runs from system_wq while the handler runs on gc->service_wq:

mana_schedule_serv_work()

	schedule_work(&mns_wk->serv_work)   /* system_wq */
	  mana_serv_reset()
	    mana_gd_suspend() -> mana_rdma_remove()
	    mana_gd_resume()  -> mana_gd_setup()
	                      -> mana_probe()
	                      -> mana_rdma_probe()

and mana_gd_resume() calls mana_gd_setup() first, which re-creates gc->service_wq, re-arms the HWC IRQs, re-creates the HW channel and re-detects devices so gc->mana_ib.dev_id.type is valid again, all before mana_rdma_probe() runs. So an event arriving during probe reaches mana_hwc_init_event_handler() -> mana_rdma_service_event() -> queue_work(gc->service_wq) and mana_rdma_service_handle() can execute on another CPU while mana_rdma_probe() is still in add_adev(). Since the event timing is driven by the host, is that window reachable in practice?

In the opposite interleaving, the handler reads gd->adev with a plain load and immediately destroys the object through remove_adev() (auxiliary_device_delete/uninit, mana_adev_idx_free) while probe is still finishing. The new acquire load only orders is_suspended against the release; the "gd->adev = adev;" store has no release pairing with the handler's load. Would publishing gd->adev with release semantics, or moving the smp_store_release() below a successful add_adev(), cover this too?

There is also the question of what rdma_teardown now is. Before this patch it was a write-once teardown flag; with the clear in mana_rdma_probe() it becomes a gate that mana_rdma_remove() closes on entry:

	WRITE_ONCE(gd->rdma_teardown, true);

	if (gc->service_wq)
		flush_workqueue(gc->service_wq);

and that mana_rdma_probe() re-opens on the way back in. That is a flag guarding a code region rather than expressing device state. Would a mutex held across mana_rdma_probe(), mana_rdma_remove() and mana_rdma_service_handle() be a better fit here, since it would also remove the window the comment documents instead of describing it?

--
Sashiko AI
review · https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%
2Fnetdev- ai.bots.linux.dev%2Fsashiko%2F%23%2Fpatchset%2F20260902175153.341056 0-1- longli%2540microsoft.com&data=05%7C02%7Clongli%40microsoft.com%7Cf8 5ef293a80341ee9ed308df0b8adad7%7C72f988bf86f141af91ab2d7cd011db47 %7C1%7C0%7C639242363166629979%7CUnknown%7CTWFpbGZsb3d8eyJFbX B0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFp bCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=nn4%2FxPu1A7bHbISZYnIuG xaoXjh%2FkS1prtwKZ4RSKao%3D&reserved=0