Re: [PATCH net-next v10 3/4] net: mana: support concurrent HWC requests

[Wei Hu]

Thanks for the review.

I will respin the series as v11 to address the admission/teardown
interaction below.

> Can this reject teardown commands without ever posting them to the
> device?

Agreed. With no available slot, abandonment or cancellation can return
-ETIMEDOUT before SQ posting, while existing destructive callers can still
free the associated memory. Their error handling predates this series, but
the new quarantine-related no-post condition must be reconciled with it.

I will address this in the next revision while preserving the lifetime
protection for timed-out IDs and buffers. Releasing a quarantined slot
merely to submit another command would reintroduce the late-response and
pending-DMA hazards.

The rejection is not permanent for every later command: a late response
can release a quarantined slot, and free slots remain usable. However,
that does not remove the unavailable-channel teardown concern. The
source-extracted tests reproduce the no-post/free control flow; they do
not establish that physical DMA corruption occurred.

> Only -EBUSY keeps a caller waiting here. mana_hwc_try_get_msg_index()
> turns a full bitmap into -ETIMEDOUT whenever abandoned_msgs is non-zero,
> however many of the occupied slots are actually abandoned.

This mixed-pool case is real. One abandoned slot does not imply that the
other occupied slots cannot complete normally.

I will cover it in the same admission revision, so partial abandonment
alone does not rule out progress from those other slots. Cancellation,
channel stop, and exhaustion of usable capacity still need to wake or
terminate waiters without reusing quarantined IDs. I will not treat an
abandoned-count comparison alone as a verified fix.

The revision will include focused regression cases for unavailable-channel
teardown, mixed healthy/abandoned occupancy, full quarantine, cancellation,
and late-response recovery.

> This isn't a bug, but the quarantine here also fixes a real misdelivery.

Agreed. I will name the original wrong-command response/misdelivery bug
explicitly and include the verified Fixes lineage and backport dependency
guidance. This is a commit-description/metadata follow-up; it does not
require an additional quarantine mechanism.

> Will this also hide live-channel failures during setup?

The specific "test_eq failed" diagnostic is suppressed for the private
bootstrap context. The final patch's outer HWC test still logs a generic
failure, so setup failure is not wholly silent.

Keeping the specific diagnostic is a small, useful fix. I will preserve it
using the explicitly passed setup context, without publishing the HWC
early or weakening the intended suppression after runtime unpublication.

> Should gc->hwc_drain_waitq be initialized once in mana_gd_probe() and
> mana_cdx_gd_probe(), next to spin_lock_init(&gc->hwc_lock)? It belongs to
> gdma_context, but here it is re-initialized on every channel creation,
> including each mana_gd_resume()->mana_gd_setup().

Agreed. I will move this initialization to gdma_context creation beside
hwc_lock in both PCI and CDX probe paths. The per-context waitqueue should
not be reinitialized by each channel constructor. This is a small
placement fix, not a broader device-lifecycle serialization change.

> This is a pre-existing issue, but the new comment claims an exclusion
> that nothing seems to enforce. Is anything keeping hwc alive between
> this read and the later mana_hwc_timeout_read() and
> mana_hwc_timeout_update() calls?

The wider raw-reader/setup/PM/service lifetime and serialization issue is
pre-existing. We will address it separately. Moving the per-context
waitqueue initialization does not claim to resolve those wider races.

Regards,
Wei

pw-bot: cr