From be34f21609e00bb8a93e4630361981648aafa4cc Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Tue, 26 Mar 2013 21:19:26 +0000 Subject: [PATCH] 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 --- sys/dev/nvme/nvme.c | 4 ++- sys/dev/nvme/nvme_ctrlr.c | 35 +++++++++---------------- sys/dev/nvme/nvme_private.h | 5 +--- sys/dev/nvme/nvme_sysctl.c | 52 ++++++++++++++++++------------------- 4 files changed, 41 insertions(+), 55 deletions(-) diff --git a/sys/dev/nvme/nvme.c b/sys/dev/nvme/nvme.c index cd2a7a6c624..27870106bf4 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 20115c6c065..4460ae8e164 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; +void +nvme_ctrlr_start_config_hook(void *arg) +{ + struct nvme_controller *ctrlr = arg; -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); - } - - 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 4a02a1b443f..0dfa2f9e26e 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 6e0aa0c8432..cabc389062e 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_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, + "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, + "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_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, + "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, + "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");