linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: keith.busch@intel.com (Busch, Keith)
Subject: [PATCH 8/9] nvme: remove dead controllers from a work item
Date: Thu, 22 Oct 2015 20:36:50 +0000	[thread overview]
Message-ID: <20151022203650.GF21840@localhost.localdomain> (raw)
In-Reply-To: <20151022181240.GA22482@lst.de>

On Thu, Oct 22, 2015@08:12:40PM +0200, Christoph Hellwig wrote:
> Oops - I'm queing it both on the system workqueue and nvme_workq, which
> is clearly broken.  I'll fix it in the next resend.

Ha, let's just get rid of nvme_workq.

I've got that plus a couple other fixups below. I noticed that earlier
in the recent changes (18/18 from the previous series) we lost the
"device_remove_file()" on the reset controller attribute. We also don't
want to leave the nvme management handle available when we're trying to
release it, so we need to delete the sysfs and char dev immediately on
removal instead of after all references are released.

This fixes a WARN in fs/sysfs/group.c line 222.

Here's all my "fixes", also pushed to my linux-nvme master:
---
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f4075f1..ebeb797 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -919,18 +919,22 @@ static void nvme_release_instance(struct nvme_ctrl *ctrl)
 	spin_unlock(&dev_list_lock);
 }
 
-static void nvme_free_ctrl(struct kref *kref)
+void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 {
-	struct nvme_ctrl *ctrl = container_of(kref, struct nvme_ctrl, kref);
+	device_remove_file(ctrl->device, &dev_attr_reset_controller);
+	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
 
 	spin_lock(&dev_list_lock);
 	list_del(&ctrl->node);
 	spin_unlock(&dev_list_lock);
+}
+
+static void nvme_free_ctrl(struct kref *kref)
+{
+	struct nvme_ctrl *ctrl = container_of(kref, struct nvme_ctrl, kref);
 
 	put_device(ctrl->device);
 	nvme_release_instance(ctrl);
-	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
-
 	ctrl->ops->free_ctrl(ctrl);
 }
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1658048..a879f9e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -189,6 +189,7 @@ static inline int nvme_error_status(u16 status)
 	}
 }
 
+void nvme_uninit_ctrl(struct nvme_ctrl *ctrl);
 int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 		const struct nvme_ctrl_ops *ops, u16 vendor,
 		unsigned long quirks);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 437c3dc..e20952d 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -69,7 +69,6 @@ MODULE_PARM_DESC(use_cmb_sqes, "use controller's memory buffer for I/O SQes");
 
 static LIST_HEAD(dev_list);
 static struct task_struct *nvme_thread;
-static struct workqueue_struct *nvme_workq;
 static wait_queue_head_t nvme_kthread_wait;
 
 struct nvme_dev;
@@ -1057,12 +1056,10 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 	 * the admin queue.
 	 */
 	if (!nvmeq->qid || cmd_rq->aborted) {
-		if (queue_work(nvme_workq, &dev->reset_work)) {
+		if (schedule_work(&dev->reset_work))
 			dev_warn(dev->dev,
 				 "I/O %d QID %d timeout, reset controller\n",
 				 req->tag, nvmeq->qid);
-		}
-
 		req->errors = -EIO;
 		return BLK_EH_HANDLED;
 	}
@@ -1563,7 +1560,7 @@ static int nvme_kthread(void *data)
 
 			if ((dev->subsystem && (csts & NVME_CSTS_NSSRO)) ||
 							csts & NVME_CSTS_CFS) {
-				if (queue_work(nvme_workq, &dev->reset_work)) {
+				if (schedule_work(&dev->reset_work)) {
 					dev_warn(dev->dev,
 						"Failed status: %x, reset controller\n",
 						readl(dev->bar + NVME_REG_CSTS));
@@ -2284,7 +2281,7 @@ static int nvme_reset(struct nvme_dev *dev)
 	if (!dev->ctrl.admin_q || blk_queue_dying(dev->ctrl.admin_q))
 		return -ENODEV;
 
-	if (!queue_work(nvme_workq, &dev->reset_work))
+	if (!schedule_work(&dev->reset_work))
 		return -EBUSY;
 
 	flush_work(&dev->reset_work);
@@ -2412,6 +2409,7 @@ static void nvme_remove(struct pci_dev *pdev)
 	nvme_remove_namespaces(&dev->ctrl);
 	nvme_dev_shutdown(dev);
 	nvme_dev_remove_admin(dev);
+	nvme_uninit_ctrl(&dev->ctrl);
 	nvme_free_queues(dev, 0);
 	nvme_release_cmb(dev);
 	nvme_release_prp_pools(dev);
@@ -2485,13 +2483,9 @@ static int __init nvme_init(void)
 
 	init_waitqueue_head(&nvme_kthread_wait);
 
-	nvme_workq = create_singlethread_workqueue("nvme");
-	if (!nvme_workq)
-		return -ENOMEM;
-
 	result = nvme_core_init();
 	if (result < 0)
-		goto kill_workq;
+		return result;
 
 	result = pci_register_driver(&nvme_driver);
 	if (result)
@@ -2500,8 +2494,6 @@ static int __init nvme_init(void)
 
  core_exit:
 	nvme_core_exit();
- kill_workq:
-	destroy_workqueue(nvme_workq);
 	return result;
 }
 
@@ -2509,7 +2501,6 @@ static void __exit nvme_exit(void)
 {
 	pci_unregister_driver(&nvme_driver);
 	nvme_core_exit();
-	destroy_workqueue(nvme_workq);
 	BUG_ON(nvme_thread && !IS_ERR(nvme_thread));
 	_nvme_check_size();
 }
--

  reply	other threads:[~2015-10-22 20:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-22 12:03 nvme abort and reset fixes Christoph Hellwig
2015-10-22 12:03 ` [PATCH 1/9] nvme: only add a controller to dev_list after it's been fully initialized Christoph Hellwig
2015-10-22 12:03 ` [PATCH 2/9] nvme: don't take the I/O queue q_lock in nvme_timeout Christoph Hellwig
2015-10-22 12:03 ` [PATCH 3/9] nvme: merge nvme_abort_req and nvme_timeout Christoph Hellwig
2015-10-22 12:03 ` [PATCH 4/9] nvme: do not restart the request timeout if we're resetting the controller Christoph Hellwig
2015-10-22 16:27   ` Busch, Keith
2015-10-22 16:30     ` Christoph Hellwig
2015-10-22 17:15       ` Busch, Keith
2015-10-22 18:17         ` Christoph Hellwig
2015-10-22 12:03 ` [PATCH 5/9] nvme: simplify resets Christoph Hellwig
2015-10-22 12:03 ` [PATCH 6/9] nvme: abort requests on the reqeueue list when shutting down a controller Christoph Hellwig
2015-10-22 14:44   ` Busch, Keith
2015-10-22 14:58     ` Christoph Hellwig
2015-10-22 15:16       ` Busch, Keith
2015-10-22 16:27         ` Christoph Hellwig
2015-10-22 12:03 ` [PATCH 7/9] nvme: merge probe_work and reset_work Christoph Hellwig
2015-10-22 12:03 ` [PATCH 8/9] nvme: remove dead controllers from a work item Christoph Hellwig
2015-10-22 18:10   ` Busch, Keith
2015-10-22 18:12     ` Christoph Hellwig
2015-10-22 20:36       ` Busch, Keith [this message]
2015-10-23  5:57         ` Christoph Hellwig
2015-10-23 14:51           ` Busch, Keith
2015-10-23 19:31             ` Busch, Keith
2015-10-22 12:03 ` [PATCH 9/9] nvme: switch abort_limit to an atomic_t Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151022203650.GF21840@localhost.localdomain \
    --to=keith.busch@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).