From patchwork Tue Jul 19 21:36:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 650478 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3rvD0J5yb0z9ssP; Wed, 20 Jul 2016 07:37:20 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical-com.20150623.gappssmtp.com header.i=@canonical-com.20150623.gappssmtp.com header.b=CWlbxzvS; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1bPcha-0000mR-5D; Tue, 19 Jul 2016 21:37:18 +0000 Received: from mail-it0-f53.google.com ([209.85.214.53]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1bPchQ-0000jf-JE for kernel-team@lists.ubuntu.com; Tue, 19 Jul 2016 21:37:08 +0000 Received: by mail-it0-f53.google.com with SMTP id f6so101631939ith.1 for ; Tue, 19 Jul 2016 14:37:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=dkY0e27Uy0SsvDJWwBWM2TAvme0JBk3W7/EjlDBWm8s=; b=CWlbxzvSL9+klVJ9Ebhk4Kgt0RhRWMX2+pcOAaOuwoGM+skKXzsI/vbfKu9X1qbpbK lLrkr2UU/jG7lYfCN6L6OZUPnFYRug9lxljGcIwkF8nKdTEwbk6RqJ9oWyuvoiI2umGy 34s/qpF4zDb53/mAG2jskSBJHJtZJNWS/biNe6Lqea0KRTtniz9NwwM5MB2FMw7TXjvD I3/hl5gW9tJz/ncc2xv7S2w6eeyxoxPPYnhjZXADqArXsVLoWBIWQYB4XAZcalEPGE5/ y8geOy7XhgSSLQohgL/YVVVBL1rOGlvodE5xdwzrLIFK/5PbRmbObfYRXBqw1zkXOFjQ S5zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=dkY0e27Uy0SsvDJWwBWM2TAvme0JBk3W7/EjlDBWm8s=; b=QEe5NnP5Xzf5YJhPI/PFwGEHt2o4hjKo74EecrG5GqxxO6W9wluHeoPhcBQG4zXchC 0oTdXzQGGlNZXZvGMZlbtiHi/izWuNnPu7c3gTC/zRxWCSaRl+y9WPg5yrmVfrKPynMa xXVkRTDoXAEfw+m00GV0osq657V6+izHcJukEcDNw56Zxfitnb5n3ofnmqAXmxN9QTFX 5WT9wkneqM15SJ3/Jbi8lBBFWQL9FCeWih/FLmUZ2z2MCBCtIAR5yoCe//GW55GR251s hbV0rW09EwVpY+gwxTd5p7jFaA5KNp/gQpa+i5suIVK8RXCYfXJDh0p+Ah1LinLAI9as tN9A== X-Gm-Message-State: ALyK8tJYtPdqyegu4UpnNLa8umVpJICVDzK4/TFobO4sh50e7toaw0zY35+thvfEGI3HgbRN X-Received: by 10.36.86.134 with SMTP id o128mr6847727itb.5.1468964227271; Tue, 19 Jul 2016 14:37:07 -0700 (PDT) Received: from localhost.localdomain (host-174-45-44-32.hln-mt.client.bresnan.net. [174.45.44.32]) by smtp.gmail.com with ESMTPSA id d126sm11610116iog.20.2016.07.19.14.37.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 19 Jul 2016 14:37:06 -0700 (PDT) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH 3/6 Xenial SRU V2] nvme: replace the kthread with a per-device watchdog timer Date: Tue, 19 Jul 2016 15:36:56 -0600 Message-Id: <1468964219-28023-4-git-send-email-tim.gardner@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1468964219-28023-1-git-send-email-tim.gardner@canonical.com> References: <20160719180800.GB13108@whence.com> <1468964219-28023-1-git-send-email-tim.gardner@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com From: Christoph Hellwig BugLink: http://bugs.launchpad.net/bugs/1602724 The only work left in the kthread is the periodic health check for each controller. There is no need to run this from process context or keep a thread context around for it, so replace it with a simpler timer. Signed-off-by: Christoph Hellwig Reviewed-by: Keith Busch Reviewed-by: Sagi Grimberg Reviewed-by: Johannes Thumshirn Signed-off-by: Jens Axboe (back ported from commit 2d55cd5f511d6fc377734473b237ac50820bfb9f) Signed-off-by: Tim Gardner Conflicts: drivers/nvme/host/pci.c --- drivers/nvme/host/pci.c | 111 ++++++++++-------------------------------------- 1 file changed, 23 insertions(+), 88 deletions(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index f1069d9..e338ce2 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -39,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -76,10 +76,7 @@ static bool use_cmb_sqes = true; module_param(use_cmb_sqes, bool, 0644); 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; struct nvme_queue; @@ -93,7 +90,6 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown); * Represents an NVM Express device. Each nvme_dev is a PCI function. */ struct nvme_dev { - struct list_head node; struct nvme_queue **queues; struct blk_mq_tag_set tagset; struct blk_mq_tag_set admin_tagset; @@ -112,6 +108,7 @@ struct nvme_dev { struct work_struct scan_work; struct work_struct remove_work; struct work_struct async_work; + struct timer_list watchdog_timer; struct mutex shutdown_lock; bool subsystem; void __iomem *cmb; @@ -1369,36 +1366,26 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev) return result; } -static int nvme_kthread(void *data) +static void nvme_watchdog_timer(unsigned long data) { - struct nvme_dev *dev, *next; + struct nvme_dev *dev = (struct nvme_dev *)data; + u32 csts = readl(dev->bar + NVME_REG_CSTS); - while (!kthread_should_stop()) { - set_current_state(TASK_INTERRUPTIBLE); - spin_lock(&dev_list_lock); - list_for_each_entry_safe(dev, next, &dev_list, node) { - u32 csts = readl(dev->bar + NVME_REG_CSTS); - - /* - * Skip controllers currently under reset. - */ - if (work_pending(&dev->reset_work) || work_busy(&dev->reset_work)) - continue; - - if ((dev->subsystem && (csts & NVME_CSTS_NSSRO)) || - csts & NVME_CSTS_CFS) { - if (queue_work(nvme_workq, &dev->reset_work)) { - dev_warn(dev->dev, - "Failed status: %x, reset controller\n", - readl(dev->bar + NVME_REG_CSTS)); - } - continue; - } + /* + * Skip controllers currently under reset. + */ + if (!work_pending(&dev->reset_work) && !work_busy(&dev->reset_work) && + ((csts & NVME_CSTS_CFS) || + (dev->subsystem && (csts & NVME_CSTS_NSSRO)))) { + if (queue_work(nvme_workq, &dev->reset_work)) { + dev_warn(dev->dev, + "Failed status: 0x%x, reset controller.\n", + csts); } - spin_unlock(&dev_list_lock); - schedule_timeout(round_jiffies_relative(HZ)); + return; } - return 0; + + mod_timer(&dev->watchdog_timer, round_jiffies(jiffies + HZ)); } static int nvme_create_io_queues(struct nvme_dev *dev) @@ -1807,56 +1794,12 @@ static void nvme_dev_unmap(struct nvme_dev *dev) } } -static int nvme_dev_list_add(struct nvme_dev *dev) -{ - bool start_thread = false; - - spin_lock(&dev_list_lock); - if (list_empty(&dev_list) && IS_ERR_OR_NULL(nvme_thread)) { - start_thread = true; - nvme_thread = NULL; - } - list_add(&dev->node, &dev_list); - spin_unlock(&dev_list_lock); - - if (start_thread) { - nvme_thread = kthread_run(nvme_kthread, NULL, "nvme"); - wake_up_all(&nvme_kthread_wait); - } else - wait_event_killable(nvme_kthread_wait, nvme_thread); - - if (IS_ERR_OR_NULL(nvme_thread)) - return nvme_thread ? PTR_ERR(nvme_thread) : -EINTR; - - return 0; -} - -/* -* Remove the node from the device list and check -* for whether or not we need to stop the nvme_thread. -*/ -static void nvme_dev_list_remove(struct nvme_dev *dev) -{ - struct task_struct *tmp = NULL; - - spin_lock(&dev_list_lock); - list_del_init(&dev->node); - if (list_empty(&dev_list) && !IS_ERR_OR_NULL(nvme_thread)) { - tmp = nvme_thread; - nvme_thread = NULL; - } - spin_unlock(&dev_list_lock); - - if (tmp) - kthread_stop(tmp); -} - static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown) { int i; u32 csts = -1; - nvme_dev_list_remove(dev); + del_timer_sync(&dev->watchdog_timer); mutex_lock(&dev->shutdown_lock); if (dev->bar) { @@ -1957,9 +1900,7 @@ static void nvme_reset_work(struct work_struct *work) dev->ctrl.event_limit = NVME_NR_AEN_COMMANDS; queue_work(nvme_workq, &dev->async_work); - result = nvme_dev_list_add(dev); - if (result) - goto remove; + mod_timer(&dev->watchdog_timer, round_jiffies(jiffies + HZ)); /* * Keep the controller around but remove all namespaces if we don't have @@ -1976,8 +1917,6 @@ static void nvme_reset_work(struct work_struct *work) clear_bit(NVME_CTRL_RESETTING, &dev->flags); return; - remove: - nvme_dev_list_remove(dev); free_tags: nvme_dev_remove_admin(dev); blk_put_queue(dev->ctrl.admin_q); @@ -2086,11 +2025,12 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) dev->dev = get_device(&pdev->dev); pci_set_drvdata(pdev, dev); - INIT_LIST_HEAD(&dev->node); INIT_WORK(&dev->scan_work, nvme_dev_scan); INIT_WORK(&dev->reset_work, nvme_reset_work); INIT_WORK(&dev->remove_work, nvme_remove_dead_ctrl_work); INIT_WORK(&dev->async_work, nvme_async_event_work); + setup_timer(&dev->watchdog_timer, nvme_watchdog_timer, + (unsigned long)dev); mutex_init(&dev->shutdown_lock); init_completion(&dev->ioq_wait); @@ -2137,9 +2077,7 @@ static void nvme_remove(struct pci_dev *pdev) { struct nvme_dev *dev = pci_get_drvdata(pdev); - spin_lock(&dev_list_lock); - list_del_init(&dev->node); - spin_unlock(&dev_list_lock); + del_timer_sync(&dev->watchdog_timer); set_bit(NVME_CTRL_REMOVING, &dev->flags); pci_set_drvdata(pdev, NULL); @@ -2253,8 +2191,6 @@ static int __init nvme_init(void) { int result; - init_waitqueue_head(&nvme_kthread_wait); - nvme_workq = alloc_workqueue("nvme", WQ_UNBOUND | WQ_MEM_RECLAIM, 0); if (!nvme_workq) return -ENOMEM; @@ -2280,7 +2216,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(); }