git: 0d2cce768cbc - stable/14 - mpi3mr: Don't hold fwevt_lock over call to taskqueue_drain

From: Alexander Motin <mav_at_FreeBSD.org>
Date: Fri, 19 Jan 2024 17:17:28 UTC
The branch stable/14 has been updated by mav:

URL: https://cgit.FreeBSD.org/src/commit/?id=0d2cce768cbcdd97efd027661c53611c122492ce

commit 0d2cce768cbcdd97efd027661c53611c122492ce
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2023-11-29 01:48:48 +0000
Commit:     Alexander Motin <mav@FreeBSD.org>
CommitDate: 2024-01-19 17:16:49 +0000

    mpi3mr: Don't hold fwevt_lock over call to taskqueue_drain
    
    Holding fwevt_lock when we call taskqueue_drain can lead to deadlock
    because it's draining a queue needs fwevt_lock to do work, so that other
    thread will try to take out the lock and block, making the thread never
    finish and taskqueue_drain never complete. There's a witness
    warning/error for this which was exposed when the lock was converted to
    a MTX_DEF lock from a MTX_SPIN prior to committing to the FreeBSD tree.
    
    The lock appears to be to protect against additional items being added
    to the event list while we're doing a reset. Since the taskqueue is
    blocked, items can get added to the list, but won't be processed during
    the reset, but there is still a (likely small) race between the
    taskqueue_drain and the taskqueue_block calls where an interrupt could
    fire on another CPU, resulting in a task being enqueued and started
    before the block can take effect. The only way to fix that race is to
    turn off interrupt processing during a reset. So we replace a deadlock
    with a smaller race.
    
    Sponsored by:           Netflix
    Reviewed by:            sumit.saxena_broadcom.com, mav, jhb
    Differential Revision:  https://reviews.freebsd.org/D42537
    
    (cherry picked from commit b411372b7d17ae7e5d6c944732d41b979bde2ac4)
---
 sys/dev/mpi3mr/mpi3mr.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sys/dev/mpi3mr/mpi3mr.c b/sys/dev/mpi3mr/mpi3mr.c
index 86535383b8f0..592b0f02cdd7 100644
--- a/sys/dev/mpi3mr/mpi3mr.c
+++ b/sys/dev/mpi3mr/mpi3mr.c
@@ -2785,7 +2785,6 @@ int mpi3mr_initialize_ioc(struct mpi3mr_softc *sc, U8 init_type)
 		mtx_init(&sc->sense_buf_q_lock, "Sense buffer Queue lock", NULL, MTX_SPIN);
 		mtx_init(&sc->chain_buf_lock, "Chain buffer lock", NULL, MTX_SPIN);
 		mtx_init(&sc->cmd_pool_lock, "Command pool lock", NULL, MTX_DEF);
-//		mtx_init(&sc->fwevt_lock, "Firmware Event lock", NULL, MTX_SPIN);
 		mtx_init(&sc->fwevt_lock, "Firmware Event lock", NULL, MTX_DEF);
 		mtx_init(&sc->target_lock, "Target lock", NULL, MTX_SPIN);
 		mtx_init(&sc->reset_mutex, "Reset lock", NULL, MTX_DEF);
@@ -5826,11 +5825,14 @@ static int mpi3mr_issue_reset(struct mpi3mr_softc *sc, U16 reset_type,
 
 inline void mpi3mr_cleanup_event_taskq(struct mpi3mr_softc *sc)
 {
-	mtx_lock(&sc->fwevt_lock);
-	taskqueue_drain(sc->cam_sc->ev_tq, &sc->cam_sc->ev_task);
+	/*
+	 * Block the taskqueue before draining. This means any new tasks won't
+	 * be queued to a worker thread. But it doesn't stop the current workers
+	 * that are running. taskqueue_drain waits for those correctly in the
+	 * case of thread backed taskqueues.
+	 */
 	taskqueue_block(sc->cam_sc->ev_tq);
-	mtx_unlock(&sc->fwevt_lock);
-	return;
+	taskqueue_drain(sc->cam_sc->ev_tq, &sc->cam_sc->ev_task);
 }
 
 /**