On Tue, 10 Nov 2026 09:42:43 +0000 Orgad Shaneh <[email protected]> wrote:
> Rx interrupt enable and disable wait for reset_ops_lock and then > return -EBUSY if a reset is in progress. The reset thread holds that > lock across the teardown (mana_dev_stop(), the RESET_ENTER request > to secondary processes, mana_dev_close()) and across the re-probe, > and each request to the secondaries may take up to > MANA_MP_REQ_TIMEOUT_SEC. An lcore that calls rx_intr_enable or > rx_intr_disable during one of those steps blocks for that long, > only to get -EBUSY. > > Retry the trylock while the device is active, and return -EBUSY as > soon as it is not. Outside a reset the lock is held only by another > queue's doorbell write or event read, so the wait stays short; once > a reset has started, the caller returns at once. > > Suggested-by: Wei Hu <[email protected]> > Signed-off-by: Orgad Shaneh <[email protected]> > --- More detailed AI review of this. Subject: Re: [PATCH] net/mana: do not block Rx interrupt calls during a reset The reset-path logic is right. mana_intr_handler() stores MANA_DEV_RESET_ENTER under reset_ops_lock before the reset thread starts, the state stays non-ACTIVE through teardown (RESET_ENTER) and re-probe (RESET_EXIT), and ACTIVE is only stored at the end of re-probe with the lock still held. The relaxed load in the loop is only a hint; the acquire load after the lock is taken is the one that matters. The PCI remove path stores RESET_FAILED, which also ends the spin. Warning The commit message and the new comment say that outside a reset the lock is held only by another queue's doorbell write or event read. That is not true. mana_dev_stop_lock() and mana_dev_close_lock() take reset_ops_lock with dev_state still MANA_DEV_ACTIVE and hold it across mana_dev_stop(), which calls mana_mp_req_on_rxtx(dev, MANA_MP_REQ_STOP_RXTX). That request can wait MANA_MP_REQ_TIMEOUT_SEC (5 s). dev_state does not change, so an lcore in rx_intr_enable/disable now busy-spins with rte_pause() for up to 5 s, where before it slept in pthread_mutex_lock(). The control ops that use mana_reset_trylock() also hold the lock from a control thread that can be preempted. rte_pause() is only a CPU hint ("pause" on x86). It gives pipeline resources to the SMT sibling, not the CPU to another thread. If the lock holder is preempted on the same CPU (a control thread or a secondary's thread sharing the lcore's core), the spinner keeps the CPU until its time slice runs out, and the holder cannot release the lock until then. Yield instead: while (pthread_mutex_trylock(&priv->reset_ops_lock)) { if (rte_atomic_load_explicit(&priv->dev_state, rte_memory_order_relaxed) != MANA_DEV_ACTIVE) return -EBUSY; sched_yield(); } Use sched_yield() rather than pthread_yield(), which is a GNU extension deprecated since glibc 2.34. Calling rx_intr_* concurrently with dev_stop is an application bug. But the wait is not "short", and the commit message should not say it is. Either fix the wording: Outside a reset the lock is held by another queue's Rx interrupt call or by a control operation. The data path must already be stopped before dev_stop/dev_close, so the wait is short in a correct application. and the same in the comment above mana_reset_lock(). Or stop the queues from contending with each other in the first place. The only reason rx_intr takes the device-wide lock is to exclude teardown. A process-shared pthread_rwlock_t would do that without the spin: rx_intr takes the read lock with tryrdlock and returns -EBUSY on failure, and control ops and the reset thread take the write lock. Queues never block each other, and a writer of any kind fails the call immediately. That fits better with the burst_state follow-up for main than with this patch, so the wording fix is enough here.
