Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Lockless access to `gd->adev` during probe causes hardware SUSPEND events to be silently dropped, leading to a state mismatch between the host and hardware. --
commit a413863cad4d3b91f96dce2e01b3fa3e47e6054e Author: Long Li <[email protected]> net: mana: Clear RDMA teardown and suspend state in mana_rdma_probe() This commit clears gd->rdma_teardown and gd->is_suspended during mana_rdma_probe() to allow servicing events to be handled again after a reset cycle. It uses smp_store_release() to publish the state before adding the auxiliary device. > 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 [ ... ] > @@ -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"); [Severity: High] Could clearing the teardown flag before the auxiliary device is fully initialized cause hardware suspend events to be silently dropped? If a GDMA_SERVICE_TYPE_RDMA_SUSPEND event arrives from hardware immediately after smp_store_release() clears gd->rdma_teardown, the handler in mana_rdma_service_handle() will be able to process it. However, because add_adev() has not yet completed and published gd->adev, the lockless check in mana_rdma_service_handle() will see a NULL pointer: mana_rdma_service_handle() { ... switch (serv_work->event) { case GDMA_SERVICE_TYPE_RDMA_SUSPEND: if (!gd->adev || gd->is_suspended) break; ... } Will this result in the handler breaking out early and dropping the suspend event locklessly, leading to a state mismatch between the host and hardware? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
