Remove the is_started flag from struct nvme_controller.

This flag was originally added to communicate to the sysctl code
which oids should be built, but there are easier ways to do this.  This
needs to be cleaned up prior to adding new controller states - for example,
controller failure.

Sponsored by:	Intel
Reviewed by:	carl

4 files changed, 46 insertions, 60 deletions

diff --git a/sys/dev/nvme/nvme.c b/sys/dev/nvme/nvme.c
index cd2a7a6..2787010 100644
--- a/sys/dev/nvme/nvme.c
+++ b/sys/dev/nvme/nvme.c
@@ -283,7 +283,9 @@ nvme_attach(device_t dev)
 	if (status != 0)
 		return (status);
 
-	ctrlr->config_hook.ich_func = nvme_ctrlr_start;
+	nvme_sysctl_initialize_ctrlr(ctrlr);
+
+	ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
 	ctrlr->config_hook.ich_arg = ctrlr;
 
 	config_intrhook_establish(&ctrlr->config_hook);
diff --git a/sys/dev/nvme/nvme_ctrlr.c b/sys/dev/nvme/nvme_ctrlr.c
index 20115c6..4460ae8 100644
--- a/sys/dev/nvme/nvme_ctrlr.c
+++ b/sys/dev/nvme/nvme_ctrlr.c
@@ -733,7 +733,7 @@ nvme_ctrlr_configure_int_coalescing(struct nvme_controller *ctrlr)
 	    ctrlr->int_coal_threshold, NULL, NULL);
 }
 
-void
+static void
 nvme_ctrlr_start(void *ctrlr_arg)
 {
 	struct nvme_controller *ctrlr = ctrlr_arg;
@@ -746,40 +746,31 @@ nvme_ctrlr_start(void *ctrlr_arg)
 	nvme_admin_qpair_enable(&ctrlr->adminq);
 
 	if (nvme_ctrlr_identify(ctrlr) != 0)
-		goto err;
+		return;
 
 	if (nvme_ctrlr_set_num_qpairs(ctrlr) != 0)
-		goto err;
+		return;
 
 	if (nvme_ctrlr_create_qpairs(ctrlr) != 0)
-		goto err;
+		return;
 
 	if (nvme_ctrlr_construct_namespaces(ctrlr) != 0)
-		goto err;
+		return;
 
 	nvme_ctrlr_configure_aer(ctrlr);
 	nvme_ctrlr_configure_int_coalescing(ctrlr);
 
 	for (i = 0; i < ctrlr->num_io_queues; i++)
 		nvme_io_qpair_enable(&ctrlr->ioq[i]);
+}
 
-	ctrlr->is_started = TRUE;
-
-err:
-
-	if (ctrlr->num_start_attempts == 0) {
-		/*
-		 * Initialize sysctls, even if controller failed to start, to
-		 *  assist with debugging admin queue pair.  Only run this
-		 *  code on the initial start attempt though, and not
-		 *  subsequent start attempts due to controller-level resets.
-		 *
-		 */
-		nvme_sysctl_initialize_ctrlr(ctrlr);
-		config_intrhook_disestablish(&ctrlr->config_hook);
-	}
+void
+nvme_ctrlr_start_config_hook(void *arg)
+{
+	struct nvme_controller *ctrlr = arg;
 
-	ctrlr->num_start_attempts++;
+	nvme_ctrlr_start(ctrlr);
+	config_intrhook_disestablish(&ctrlr->config_hook);
 }
 
 static void
@@ -906,8 +897,6 @@ nvme_ctrlr_construct(struct nvme_controller *ctrlr, device_t dev)
 	int			timeout_period;
 
 	ctrlr->dev = dev;
-	ctrlr->is_started = FALSE;
-	ctrlr->num_start_attempts = 0;
 
 	status = nvme_ctrlr_allocate_bar(ctrlr);
 
diff --git a/sys/dev/nvme/nvme_private.h b/sys/dev/nvme/nvme_private.h
index 4a02a1b..0dfa2f9 100644
--- a/sys/dev/nvme/nvme_private.h
+++ b/sys/dev/nvme/nvme_private.h
@@ -250,7 +250,6 @@ struct nvme_controller {
 	struct intr_config_hook	config_hook;
 	uint32_t		ns_identified;
 	uint32_t		queues_created;
-	uint32_t		num_start_attempts;
 	struct task		reset_task;
 	struct taskqueue	*taskqueue;
 
@@ -287,8 +286,6 @@ struct nvme_controller {
 
 	struct cdev			*cdev;
 
-	boolean_t			is_started;
-
 	uint32_t			num_aers;
 	struct nvme_async_event_request	aer[NVME_MAX_ASYNC_EVENTS];
 
@@ -401,7 +398,7 @@ void	nvme_ctrlr_destruct(struct nvme_controller *ctrlr, device_t dev);
 int	nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr);
 void	nvme_ctrlr_reset(struct nvme_controller *ctrlr);
 /* ctrlr defined as void * to allow use with config_intrhook. */
-void	nvme_ctrlr_start(void *ctrlr_arg);
+void	nvme_ctrlr_start_config_hook(void *ctrlr_arg);
 void	nvme_ctrlr_submit_admin_request(struct nvme_controller *ctrlr,
 					struct nvme_request *req);
 void	nvme_ctrlr_submit_io_request(struct nvme_controller *ctrlr,
diff --git a/sys/dev/nvme/nvme_sysctl.c b/sys/dev/nvme/nvme_sysctl.c
index 6e0aa0c..cabc389 100644
--- a/sys/dev/nvme/nvme_sysctl.c
+++ b/sys/dev/nvme/nvme_sysctl.c
@@ -251,38 +251,36 @@ nvme_sysctl_initialize_ctrlr(struct nvme_controller *ctrlr)
 	ctrlr_tree = device_get_sysctl_tree(ctrlr->dev);
 	ctrlr_list = SYSCTL_CHILDREN(ctrlr_tree);
 
-	if (ctrlr->is_started) {
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "int_coal_time", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
-		    nvme_sysctl_int_coal_time, "IU",
-		    "Interrupt coalescing timeout (in microseconds)");
-
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "int_coal_threshold", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
-		    nvme_sysctl_int_coal_threshold, "IU",
-		    "Interrupt coalescing threshold");
-
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "timeout_period", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
-		    nvme_sysctl_timeout_period, "IU",
-		    "Timeout period (in seconds)");
-
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "num_cmds", CTLTYPE_S64 | CTLFLAG_RD,
-		    ctrlr, 0, nvme_sysctl_num_cmds, "IU",
-		    "Number of commands submitted");
-
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "num_intr_handler_calls", CTLTYPE_S64 | CTLFLAG_RD,
-		    ctrlr, 0, nvme_sysctl_num_intr_handler_calls, "IU",
-		    "Number of times interrupt handler was invoked (will "
-		    "typically be less than number of actual interrupts "
-		    "generated due to coalescing)");
-
-		SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
-		    "reset_stats", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
-		    nvme_sysctl_reset_stats, "IU", "Reset statistics to zero");
-	}
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "int_coal_time", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
+	    nvme_sysctl_int_coal_time, "IU",
+	    "Interrupt coalescing timeout (in microseconds)");
+
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "int_coal_threshold", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
+	    nvme_sysctl_int_coal_threshold, "IU",
+	    "Interrupt coalescing threshold");
+
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "timeout_period", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
+	    nvme_sysctl_timeout_period, "IU",
+	    "Timeout period (in seconds)");
+
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "num_cmds", CTLTYPE_S64 | CTLFLAG_RD,
+	    ctrlr, 0, nvme_sysctl_num_cmds, "IU",
+	    "Number of commands submitted");
+
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "num_intr_handler_calls", CTLTYPE_S64 | CTLFLAG_RD,
+	    ctrlr, 0, nvme_sysctl_num_intr_handler_calls, "IU",
+	    "Number of times interrupt handler was invoked (will "
+	    "typically be less than number of actual interrupts "
+	    "generated due to coalescing)");
+
+	SYSCTL_ADD_PROC(ctrlr_ctx, ctrlr_list, OID_AUTO,
+	    "reset_stats", CTLTYPE_UINT | CTLFLAG_RW, ctrlr, 0,
+	    nvme_sysctl_reset_stats, "IU", "Reset statistics to zero");
 
 	que_tree = SYSCTL_ADD_NODE(ctrlr_ctx, ctrlr_list, OID_AUTO, "adminq",
 	    CTLFLAG_RD, NULL, "Admin Queue");