git: 54fa0d10f68c - stable/14 - mpi3mr: Reduce the scope of the reset_mutext

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

URL: https://cgit.FreeBSD.org/src/commit/?id=54fa0d10f68c81722c341ae4277d0c5634babd48

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

    mpi3mr: Reduce the scope of the reset_mutext
    
    Reduce the scope of reset_mutext to protect the msleep in the watch dog
    thread as well as the MPI3MR_FLAGS_SHUTDOWN bit. Use it to protect the
    wakeup in mpi3mr_detach so this thread can exit sooner when we're trying
    to do an orderly shutdown. Optimize the flow to check the sleep and
    other conditions before going to sleep.
    
    It's an open question if this should protect sc->unrecoverable, and if
    we should wakeup the watchdog thread when we set it. We might also want
    to move too booleans for the three flags that we have now in
    mpi3mr_flags. There are a number of U8s that should really be bools and
    we might want to also group them together to pack softc better.
    
    Sponsored by:           Netflix
    Reviewed by:            mav
    Differential Revision:  https://reviews.freebsd.org/D42539
    
    (cherry picked from commit 7c4913093a759adf2e4c7d65535aee04aadee4df)
---
 sys/dev/mpi3mr/mpi3mr.c     | 26 +++++++++++++++++---------
 sys/dev/mpi3mr/mpi3mr_pci.c |  4 +++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/sys/dev/mpi3mr/mpi3mr.c b/sys/dev/mpi3mr/mpi3mr.c
index 7bb834430dbc..f6c4bb4496e6 100644
--- a/sys/dev/mpi3mr/mpi3mr.c
+++ b/sys/dev/mpi3mr/mpi3mr.c
@@ -3038,9 +3038,6 @@ mpi3mr_watchdog_thread(void *arg)
 	sc->watchdog_thread_active = 1;
 	mtx_lock(&sc->reset_mutex);
 	for (;;) {
-		/* Sleep for 1 second and check the queue status */
-		msleep(&sc->watchdog_chan, &sc->reset_mutex, PRIBIO,
-		    "mpi3mr_watchdog", 1 * hz);
 		if (sc->mpi3mr_flags & MPI3MR_FLAGS_SHUTDOWN || 
 		    (sc->unrecoverable == 1)) {
 			mpi3mr_dprint(sc, MPI3MR_INFO,
@@ -3049,20 +3046,21 @@ mpi3mr_watchdog_thread(void *arg)
 			    "Hardware critical error", __func__);
 			break;
 		}
+		mtx_unlock(&sc->reset_mutex);
 
 		if ((sc->prepare_for_reset) &&
 		    ((sc->prepare_for_reset_timeout_counter++) >=
 		     MPI3MR_PREPARE_FOR_RESET_TIMEOUT)) {
 			mpi3mr_soft_reset_handler(sc,
 			    MPI3MR_RESET_FROM_CIACTVRST_TIMER, 1);
-			continue;
+			goto sleep;
 		}
 	
 		ioc_status = mpi3mr_regread(sc, MPI3_SYSIF_IOC_STATUS_OFFSET);
 		
 		if (ioc_status & MPI3_SYSIF_IOC_STATUS_RESET_HISTORY) {
 			mpi3mr_soft_reset_handler(sc, MPI3MR_RESET_FROM_FIRMWARE, 0);
-			continue;
+			goto sleep;
 		}
 
 		ioc_state = mpi3mr_get_iocstate(sc);
@@ -3078,7 +3076,7 @@ mpi3mr_watchdog_thread(void *arg)
 						"diag save in progress\n");
 				}
 				if ((sc->diagsave_timeout++) <= MPI3_SYSIF_DIAG_SAVE_TIMEOUT)
-					continue;
+					goto sleep;
 			}
 			mpi3mr_print_fault_info(sc);
 			sc->diagsave_timeout = 0;
@@ -3089,12 +3087,12 @@ mpi3mr_watchdog_thread(void *arg)
 				    "Controller requires system power cycle or complete reset is needed,"
 				    "fault code: 0x%x. marking controller as unrecoverable\n", fault);
 				sc->unrecoverable = 1;
-				goto out;
+				break;
 			}
 			if ((fault == MPI3_SYSIF_FAULT_CODE_DIAG_FAULT_RESET)
 			    || (fault == MPI3_SYSIF_FAULT_CODE_SOFT_RESET_IN_PROGRESS)
 			    || (sc->reset_in_progress))
-				goto out;
+				break;
 			if (fault == MPI3_SYSIF_FAULT_CODE_CI_ACTIVATION_RESET)
 				mpi3mr_soft_reset_handler(sc,
 				    MPI3MR_RESET_FROM_CIACTIV_FAULT, 0);
@@ -3108,8 +3106,18 @@ mpi3mr_watchdog_thread(void *arg)
 			mpi3mr_print_fault_info(sc);
 			mpi3mr_soft_reset_handler(sc, sc->reset.reason, 1);
 		}
+sleep:
+		mtx_lock(&sc->reset_mutex);
+		/*
+		 * Sleep for 1 second if we're not exiting, then loop to top
+		 * to poll exit status and hardware health.
+		 */
+		if ((sc->mpi3mr_flags & MPI3MR_FLAGS_SHUTDOWN) == 0 &&
+		    !sc->unrecoverable) {
+			msleep(&sc->watchdog_chan, &sc->reset_mutex, PRIBIO,
+			    "mpi3mr_watchdog", 1 * hz);
+		}
 	}
-out:
 	mtx_unlock(&sc->reset_mutex);
 	sc->watchdog_thread_active = 0;
 	mpi3mr_kproc_exit(0);
diff --git a/sys/dev/mpi3mr/mpi3mr_pci.c b/sys/dev/mpi3mr/mpi3mr_pci.c
index 4935ac0d519c..eaf73022291d 100644
--- a/sys/dev/mpi3mr/mpi3mr_pci.c
+++ b/sys/dev/mpi3mr/mpi3mr_pci.c
@@ -635,13 +635,15 @@ mpi3mr_pci_detach(device_t dev)
 	if (!sc->secure_ctrl)
 		return 0;
 	
-	sc->mpi3mr_flags |= MPI3MR_FLAGS_SHUTDOWN;
 	
 	if (sc->sysctl_tree != NULL)
 		sysctl_ctx_free(&sc->sysctl_ctx);
 	
+	mtx_lock(&sc->reset_mutex);
+	sc->mpi3mr_flags |= MPI3MR_FLAGS_SHUTDOWN;
 	if (sc->watchdog_thread_active)
 		wakeup(&sc->watchdog_chan);
+	mtx_unlock(&sc->reset_mutex);
 	
 	while (sc->reset_in_progress && (i < PEND_IOCTLS_COMP_WAIT_TIME)) {
 		i++;