libie_ctlq_xn_shutdown() waits on xnm->can_destroy for the outstanding
transactions to be returned. libie_ctlq_xn_push_free() signals it with
complete() when the last entry goes back to the free bitmap. Once the
manager is shut down, libie_ctlq_xn_pop_free() hands out no more entries,
so that point is reached exactly once and only one waiter is woken.
Nothing restricts libie_ctlq_xn_shutdown() to a single caller, though.
If two threads call it while a transaction is still outstanding, both see
the entry in use and both wait, and the one that is not woken blocks
forever, as wait_for_completion() has no timeout.
A drained manager stays drained until libie_ctlq_xn_deinit() frees it, so
signal it with complete_all(). All callers waiting at that point are
released, and later ones find the bitmap full and return at once.
Signal it before free_xns_bm_lock is dropped. A caller that finds the
bitmap full then knows the completion has already been signalled. That
leaves libie_ctlq_xn_push_free() holding the lock after it has woken the
waiters, so have a woken waiter take and release free_xns_bm_lock before
it returns, the same lock/unlock pair completion_done() uses to wait for
complete(), see commit dec13c42d21a ("completion: Replace
spin_unlock_wait() with lock/unlock pair"). Its unlock is then the last
access to the manager by the time any caller returns.
The manager must still outlive every caller, since a woken waiter has to
retake the completion and bitmap locks before it returns. Spell that out
in the kernel-doc: libie_ctlq_xn_deinit() must not be called until every
libie_ctlq_xn_shutdown() caller has returned.
The next patch makes idpf call libie_ctlq_xn_shutdown() from its service
task, which can run concurrently with the reset and remove paths.
Fixes: b4ff4e626a7c ("libie: add bookkeeping support for control queue
messages")
Assisted-by: LLM
Signed-off-by: Emil Tantilov <[email protected]>
---
drivers/net/ethernet/intel/libie/controlq.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/libie/controlq.c
b/drivers/net/ethernet/intel/libie/controlq.c
index 6214fc036ce5f..9bcb022ed9782 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -735,13 +735,10 @@ static bool __libie_ctlq_xn_push_free(struct
libie_ctlq_xn_manager *xnm,
static void libie_ctlq_xn_push_free(struct libie_ctlq_xn_manager *xnm,
struct libie_ctlq_xn *xn)
{
- bool can_destroy;
-
- scoped_guard(spinlock, &xnm->free_xns_bm_lock)
- can_destroy = __libie_ctlq_xn_push_free(xnm, xn);
+ guard(spinlock)(&xnm->free_xns_bm_lock);
- if (can_destroy)
- complete(&xnm->can_destroy);
+ if (__libie_ctlq_xn_push_free(xnm, xn))
+ complete_all(&xnm->can_destroy);
}
/**
@@ -1171,7 +1168,9 @@ EXPORT_SYMBOL_NS_GPL(libie_ctlq_xn_send_clean,
"LIBIE_CP");
* @xnm: pointer to the transaction manager
*
* Synchronously terminate existing transactions and stop accepting new ones.
- * Async transactions are discarded without invoking resp_cb.
+ * Async transactions are discarded without invoking resp_cb. May be called
+ * concurrently, libie_ctlq_xn_deinit() must not be called until every caller
+ * has returned.
*/
void libie_ctlq_xn_shutdown(struct libie_ctlq_xn_manager *xnm)
{
@@ -1218,8 +1217,12 @@ void libie_ctlq_xn_shutdown(struct libie_ctlq_xn_manager
*xnm)
spin_unlock(&xnm->free_xns_bm_lock);
- if (must_wait)
+ if (must_wait) {
wait_for_completion(&xnm->can_destroy);
+ /* Synchronize with libie_ctlq_xn_push_free() unlocking */
+ spin_lock(&xnm->free_xns_bm_lock);
+ spin_unlock(&xnm->free_xns_bm_lock);
+ }
}
EXPORT_SYMBOL_NS_GPL(libie_ctlq_xn_shutdown, "LIBIE_CP");
--
2.37.3
