git: 7c4913093a75 - main - mpi3mr: Reduce the scope of the reset_mutext

The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=7c4913093a759adf2e4c7d65535aee04aadee4df

commit 7c4913093a759adf2e4c7d65535aee04aadee4df
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2023-11-29 01:49:08 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-11-29 01:49:08 +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
---
 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 9a052c98d13e..a2e43850d9b5 100644
--- a/sys/dev/mpi3mr/mpi3mr.c
+++ b/sys/dev/mpi3mr/mpi3mr.c
@@ -3037,9 +3037,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,
@@ -3048,20 +3045,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);
@@ -3077,7 +3075,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;
@@ -3088,12 +3086,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);
@@ -3107,8 +3105,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++;