From patchwork Sat Apr 29 05:35:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 9705639 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 41C29602BE for ; Sat, 29 Apr 2017 05:35:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A59B28673 for ; Sat, 29 Apr 2017 05:35:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1BBAD28684; Sat, 29 Apr 2017 05:35:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_NONE,T_DKIM_INVALID autolearn=no version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9D35628673 for ; Sat, 29 Apr 2017 05:35:26 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id CA31121951CBD; Fri, 28 Apr 2017 22:35:25 -0700 (PDT) X-Original-To: linux-nvdimm@lists.01.org Delivered-To: linux-nvdimm@lists.01.org Received: from mail-oi0-x235.google.com (mail-oi0-x235.google.com [IPv6:2607:f8b0:4003:c06::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 521A421951CB9 for ; Fri, 28 Apr 2017 22:35:24 -0700 (PDT) Received: by mail-oi0-x235.google.com with SMTP id y11so47408428oie.0 for ; Fri, 28 Apr 2017 22:35:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=2AI0YB3+5F4pnYD+A5ycjeoYZ2jbOQS7NOWNXLZPtBs=; b=ThguFbrAAI14ch43M/alQSLuEwxSMOnSUoLnB1haTkyaaVOzt5KiC5AWM2GQfUuQug fJX4MWlumFtHanUJuuCqO5AETVk0xWhstmT/PZS2wJLke4NhPZE07uedm50Gg34Wq6NO yr7L4DBggDWmJFM/wXm6oJTLQX6wNGsNcPYBNeHGJoaDx6YxRXE+QlFGOuQ5xUHGHn0J WSob7uPNikMBRf3cZf4LKtOenPprWMnzzODuVltYLAg3cVrqZ3I0nprxl/z5dX6cHlTs Qxa2hHgLwO0CDjwSO3hT5VjNGkLKxTmxUvRFKVcamviJlblVSmldCjfXPL3AQmG7eNHx U7Rw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=2AI0YB3+5F4pnYD+A5ycjeoYZ2jbOQS7NOWNXLZPtBs=; b=J2+M1f8or8o/ZHTMldsLBm2/1BvtM7IuzBEBB1uW9J1Zm/leyMEVrz0OYkKMtkVyu2 pJdQkxR4+a28QovBa+96eK0txgmWiHLGJpnHFG4hikCFhcxYnonZlRWwJk06Etan4sdk V7DzZuGf6+PFyXdDtfxDf7bsreDDuTGoKgIMPvaKmtpycWbbTcJesEWNftcStkfCUAU1 6MxmIJDBXimsKdePV979SPGzSwX6xofJK710kW9fKjjS+mWGeKB6Hg+sNZzwO5EBaIlO LW2JFLd5GzP4uB3hRBtImGEbUH2zHoWyw4imk9CaH/cMzKIa/GeZM9PYgZBQPL3R/2Yy HknA== X-Gm-Message-State: AN3rC/5msECQvKfzpu2ohpuXjgR4EzfgaQdLLeC07ehsZJBKukaQkgh/ NpqTmGRpKMjqK5sBPyLSBx9N1LS/1Z3M X-Received: by 10.202.214.131 with SMTP id n125mr5570959oig.209.1493444123385; Fri, 28 Apr 2017 22:35:23 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.34.51 with HTTP; Fri, 28 Apr 2017 22:35:22 -0700 (PDT) In-Reply-To: <1482421304.370160.1493046815781.JavaMail.zimbra@redhat.com> References: <895281518.353931.1493045976821.JavaMail.zimbra@redhat.com> <1482421304.370160.1493046815781.JavaMail.zimbra@redhat.com> From: Dan Williams Date: Fri, 28 Apr 2017 22:35:22 -0700 Message-ID: Subject: Re: task ndctl:5155 blocked for more than 120 seconds observed during pmem/btt/dax switch test To: Yi Zhang X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-nvdimm@lists.01.org" Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Apr 24, 2017 at 8:13 AM, Yi Zhang wrote: > Hello > > I reproduced ndctl blocked issue on 4.11.0-rc8, here is the reproduce steps and kernel log, could you help check it? Thanks. > > Reproduce steps: > function pmem_btt_dax_switch() { > sector_size_list="512 520 528 4096 4104 4160 4224" > for sector_size in $sector_size_list; do > ndctl create-namespace -f -e namespace${1}.0 --mode=sector -l $sector_size > ndctl create-namespace -f -e namespace${1}.0 --mode=raw > ndctl create-namespace -f -e namespace${1}.0 --mode=dax > done > } > for i in 0 1 2 3; do > pmem_btt_dax_switch $i & > done Thanks for the report! I couldn't run your script directly, do you have 4 memmap= regions defined, or...? I was able to find a locking problem with a debug patch that turned on lockdep coverage for the device_lock(). Can you give the attached patch a try to see if it resolves your lockup? This is against latest nvdimm.git/libnvdimm-for-next From c71720d805401ebfc5d53ed1c4241ec566554e60 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 28 Apr 2017 22:05:14 -0700 Subject: [PATCH] libnvdimm: fix nvdimm_bus_lock() vs device_lock() ordering A debug patch to turn the standard device_lock() into something that lockdep can analyze yielded the following: ====================================================== [ INFO: possible circular locking dependency detected ] 4.11.0-rc4+ #106 Tainted: G O ------------------------------------------------------- lt-libndctl/1898 is trying to acquire lock: (&dev->nvdimm_mutex/3){+.+.+.}, at: [] nd_attach_ndns+0x178/0x1b0 [libnvdimm] but task is already holding lock: (&nvdimm_bus->reconfig_mutex){+.+.+.}, at: [] nvdimm_bus_lock+0x21/0x30 [libnvdimm] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&nvdimm_bus->reconfig_mutex){+.+.+.}: lock_acquire+0xf6/0x1f0 __mutex_lock+0x88/0x980 mutex_lock_nested+0x1b/0x20 nvdimm_bus_lock+0x21/0x30 [libnvdimm] nvdimm_namespace_capacity+0x1b/0x40 [libnvdimm] nvdimm_namespace_common_probe+0x230/0x510 [libnvdimm] nd_pmem_probe+0x14/0x180 [nd_pmem] nvdimm_bus_probe+0xa9/0x260 [libnvdimm] -> #0 (&dev->nvdimm_mutex/3){+.+.+.}: __lock_acquire+0x1107/0x1280 lock_acquire+0xf6/0x1f0 __mutex_lock+0x88/0x980 mutex_lock_nested+0x1b/0x20 nd_attach_ndns+0x178/0x1b0 [libnvdimm] nd_namespace_store+0x308/0x3c0 [libnvdimm] namespace_store+0x87/0x220 [libnvdimm] In this case '&dev->nvdimm_mutex/3' mirrors '&dev->mutex'. Fix this by replacing the use of device_lock() with nvdimm_bus_lock() to protect nd_{attach,detach}_ndns() operations. Cc: Fixes: 8c2f7e8658df ("libnvdimm: infrastructure for btt devices") Reported-by: Yi Zhang Signed-off-by: Dan Williams --- drivers/nvdimm/btt_devs.c | 2 +- drivers/nvdimm/claim.c | 23 +++++++++++++++-------- drivers/nvdimm/dax_devs.c | 2 +- drivers/nvdimm/pfn_devs.c | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/nvdimm/btt_devs.c b/drivers/nvdimm/btt_devs.c index 97dd2925ed6e..4b76af2b8715 100644 --- a/drivers/nvdimm/btt_devs.c +++ b/drivers/nvdimm/btt_devs.c @@ -314,7 +314,7 @@ int nd_btt_probe(struct device *dev, struct nd_namespace_common *ndns) if (rc < 0) { struct nd_btt *nd_btt = to_nd_btt(btt_dev); - __nd_detach_ndns(btt_dev, &nd_btt->ndns); + nd_detach_ndns(btt_dev, &nd_btt->ndns); put_device(btt_dev); } diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 2c19bc7fc056..35b210dc1e56 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -21,8 +21,13 @@ void __nd_detach_ndns(struct device *dev, struct nd_namespace_common **_ndns) { struct nd_namespace_common *ndns = *_ndns; + struct nvdimm_bus *nvdimm_bus; - lockdep_assert_held(&ndns->dev.mutex); + if (!ndns) + return; + + nvdimm_bus = walk_to_nvdimm_bus(&ndns->dev); + lockdep_assert_held(&nvdimm_bus->reconfig_mutex); dev_WARN_ONCE(dev, ndns->claim != dev, "%s: invalid claim\n", __func__); ndns->claim = NULL; *_ndns = NULL; @@ -37,18 +42,20 @@ void nd_detach_ndns(struct device *dev, if (!ndns) return; get_device(&ndns->dev); - device_lock(&ndns->dev); + nvdimm_bus_lock(&ndns->dev); __nd_detach_ndns(dev, _ndns); - device_unlock(&ndns->dev); + nvdimm_bus_unlock(&ndns->dev); put_device(&ndns->dev); } bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, struct nd_namespace_common **_ndns) { + struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(&attach->dev); + if (attach->claim) return false; - lockdep_assert_held(&attach->dev.mutex); + lockdep_assert_held(&nvdimm_bus->reconfig_mutex); dev_WARN_ONCE(dev, *_ndns, "%s: invalid claim\n", __func__); attach->claim = dev; *_ndns = attach; @@ -61,9 +68,9 @@ bool nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, { bool claimed; - device_lock(&attach->dev); + nvdimm_bus_lock(&attach->dev); claimed = __nd_attach_ndns(dev, attach, _ndns); - device_unlock(&attach->dev); + nvdimm_bus_unlock(&attach->dev); return claimed; } @@ -114,7 +121,7 @@ static void nd_detach_and_reset(struct device *dev, struct nd_namespace_common **_ndns) { /* detach the namespace and destroy / reset the device */ - nd_detach_ndns(dev, _ndns); + __nd_detach_ndns(dev, _ndns); if (is_idle(dev, *_ndns)) { nd_device_unregister(dev, ND_ASYNC); } else if (is_nd_btt(dev)) { @@ -184,7 +191,7 @@ ssize_t nd_namespace_store(struct device *dev, } WARN_ON_ONCE(!is_nvdimm_bus_locked(dev)); - if (!nd_attach_ndns(dev, ndns, _ndns)) { + if (!__nd_attach_ndns(dev, ndns, _ndns)) { dev_dbg(dev, "%s already claimed\n", dev_name(&ndns->dev)); len = -EBUSY; diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c index 45fa82cae87c..c1b6556aea6e 100644 --- a/drivers/nvdimm/dax_devs.c +++ b/drivers/nvdimm/dax_devs.c @@ -124,7 +124,7 @@ int nd_dax_probe(struct device *dev, struct nd_namespace_common *ndns) dev_dbg(dev, "%s: dax: %s\n", __func__, rc == 0 ? dev_name(dax_dev) : ""); if (rc < 0) { - __nd_detach_ndns(dax_dev, &nd_pfn->ndns); + nd_detach_ndns(dax_dev, &nd_pfn->ndns); put_device(dax_dev); } else __nd_device_register(dax_dev); diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c index 6c033c9a2f06..c38566f4da7d 100644 --- a/drivers/nvdimm/pfn_devs.c +++ b/drivers/nvdimm/pfn_devs.c @@ -484,7 +484,7 @@ int nd_pfn_probe(struct device *dev, struct nd_namespace_common *ndns) dev_dbg(dev, "%s: pfn: %s\n", __func__, rc == 0 ? dev_name(pfn_dev) : ""); if (rc < 0) { - __nd_detach_ndns(pfn_dev, &nd_pfn->ndns); + nd_detach_ndns(pfn_dev, &nd_pfn->ndns); put_device(pfn_dev); } else __nd_device_register(pfn_dev); -- 2.9.3