Re: [PATCH net-next v7 02/13] net: mana: share the EQ pool across a queue-set swap

[Wei Hu <weh@linux.microsoft.com>]

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