mana_rdma_remove() sets gd->rdma_teardown to stop
mana_rdma_service_handle() from acting on servicing events, but nothing
ever clears it. A hardware service reset (GDMA_EQE_HWC_RESET_REQUEST)
goes through mana_gd_suspend() -> mana_rdma_remove() and mana_gd_resume()
-> mana_rdma_probe(), so from the first reset onwards every
GDMA_EQE_HWC_SOC_SERVICE event returns early and RDMA suspend/resume
servicing is silently dropped for the life of the device.

gd->is_suspended has the same problem: it is set when servicing removes
the adev and is cleared only by a matching resume. A reset while RDMA is
suspended re-adds the adev but leaves is_suspended set, so a later resume
event calls add_adev() on top of a live gd->adev and leaks it. This is
currently masked by the rdma_teardown bug.

Clear both in mana_rdma_probe(). is_suspended is otherwise only touched
by mana_rdma_service_handle() on the ordered service workqueue, so clear
it while rdma_teardown still gates that handler and re-open the gate with
smp_store_release(), paired with smp_load_acquire() in the handler.

Fixes: 505cc26bcae0 ("net: mana: Add support for auxiliary device servicing events")
Signed-off-by: Long Li <longli@microsoft.com>
---
Changes in v2:
- No functional change; the diff is identical to v1, rebased on net/main.
- Target the net tree explicitly in the subject prefix; v1 omitted it and
  the netdev CI guessed net-next, where the Fixes: tag is not evaluated.

v1: https://lore.kernel.org/all/20260826235940.1869565-1-longli@microsoft.com/
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 7a1ac853e3abcd28c4a1e5c6987ec631a18ad840..56f426da728647e39f4c4ffa9c66123436297ac1 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -3983,7 +3983,8 @@ static void mana_rdma_service_handle(struct work_struct *work)
 	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) {
@@ -4279,6 +4280,17 @@ int mana_rdma_probe(struct gdma_dev *gd)
 	if (err)
 		return err;
 
+	/* Clear the state left by a previous mana_rdma_remove() so servicing
+	 * events are handled again after a reset cycle.
+	 */
+	gd->is_suspended = false;
+
+	/* Publish is_suspended before re-opening the gate, so the handler
+	 * cannot act on a stale value.  Pairs with the smp_load_acquire()
+	 * in mana_rdma_service_handle().
+	 */
+	smp_store_release(&gd->rdma_teardown, false);
+
 	err = add_adev(gd, "rdma");
 	if (err)
 		mana_gd_deregister_device(gd);
-- 
2.43.0

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260828001202.2110938-1-longli@microsoft.com?part=1
---
net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe()

This clears gd->rdma_teardown and gd->is_suspended in mana_rdma_probe() so
that RDMA servicing events are handled again after a hardware service reset
cycle. The gate is re-opened with smp_store_release(), paired with a new
smp_load_acquire() in mana_rdma_service_handle().

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 7a1ac853e3abc..56f426da72864 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -3983,7 +3983,8 @@ static void mana_rdma_service_handle(struct work_struct *work)
>  	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) {

[ ... ]

> @@ -4279,6 +4280,17 @@ int mana_rdma_probe(struct gdma_dev *gd)
>  	if (err)
>  		return err;
>  
> +	/* Clear the state left by a previous mana_rdma_remove() so servicing
> +	 * events are handled again after a reset cycle.
> +	 */
> +	gd->is_suspended = false;

[Severity: Low]

Does the invariant described in the commit message hold on the initial probe
path?  The commit message says:

    is_suspended is otherwise only touched by mana_rdma_service_handle() on
    the ordered service workqueue, so clear it while rdma_teardown still
    gates that handler

That is true for the reset path, where mana_rdma_remove() set rdma_teardown
and flushed gc->service_wq first.  On mana_gd_probe() -> mana_probe() ->
mana_rdma_probe() the gate was never closed: gc comes from vzalloc(), so both
gd->rdma_teardown and gd->is_suspended are already false, and mana_gd_setup()
has already created gc->service_wq and brought up the HWC and EQs before
mana_rdma_probe() runs.

So on that path the smp_store_release() is a false -> false store and the
handler's smp_load_acquire() has no state change to order against, while the
plain write of gd->is_suspended here (and add_adev()'s plain write of
gd->adev) can run concurrently with the handler's plain accesses to the same
two fields with no lock on either side.

The outcome looks benign today, since is_suspended is already false and the
handler is a no-op in that window, but would it be worth wording the comment
and commit message so they only claim the property that actually holds?

> +
> +	/* Publish is_suspended before re-opening the gate, so the handler
> +	 * cannot act on a stale value.  Pairs with the smp_load_acquire()
> +	 * in mana_rdma_service_handle().
> +	 */
> +	smp_store_release(&gd->rdma_teardown, false);
> +
>  	err = add_adev(gd, "rdma");
>  	if (err)
>  		mana_gd_deregister_device(gd);

[Severity: Medium]

Is the gate re-opened too early here, i.e. before add_adev() has published
gd->adev?  add_adev() stores it as its last step, after
auxiliary_device_add() has already synchronously probed mana_ib:

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 work item that runs inside that window
passes the acquire load, but sees gd->adev == NULL because remove_adev()
cleared it during the preceding mana_rdma_remove():

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 event is consumed without setting gd->is_suspended, and the
matching GDMA_SERVICE_TYPE_RDMA_RESUME then also breaks on
!gd->is_suspended.  Doesn't that leave the freshly created RDMA auxiliary
device live across a servicing operation that explicitly asked for it to be
quiesced, which is the same silently-dropped-servicing behaviour this change
sets out to fix, now confined to the probe window?

Can the two contexts really overlap?  mana_serv_reset() runs on the system
workqueue via mana_schedule_serv_work()/schedule_work() (or from
mana_recovery_delayed_func()) and calls mana_gd_suspend() ->
mana_rdma_remove() then mana_gd_resume() -> mana_gd_setup() ->
mana_rdma_probe(), while the handler runs on gc->service_wq, which
mana_gd_setup() re-creates before mana_rdma_probe() is called:

mana_rdma_service_event() {
	...
	INIT_WORK(&serv_work->work, mana_rdma_service_handle);
	queue_work(gc->service_wq, &serv_work->work);

Different workqueues, and GC_IN_SERVICE only serializes further reset
requests.

Related to the new comment: the handler's read of gd->adev is a plain read
racing with add_adev()'s plain write, so "the handler cannot act on a stale
value" holds for is_suspended but not for gd->adev.

mana_rdma_remove() uses the opposite ordering, closing the gate and draining
the workqueue before touching gd->adev:

mana_rdma_remove() {
	WRITE_ONCE(gd->rdma_teardown, true);

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

	if (gd->adev)
		remove_adev(gd);

Would doing the mirror image in probe work here, i.e. keeping the
gd->is_suspended = false store where it is but issuing
smp_store_release(&gd->rdma_teardown, false) only after add_adev() has
succeeded?  Note that ordering alone still drops such an event, just via the
gate instead; serializing mana_rdma_probe(), mana_rdma_remove() and the
handler body with a mutex rather than extending the bool gate would close the
window entirely.