diff options
author | Dan Williams <dan.j.williams@intel.com> | 2017-04-29 01:05:14 -0400 |
---|---|---|
committer | Dan Williams <dan.j.williams@intel.com> | 2017-05-01 11:29:37 -0400 |
commit | 452bae0aede774f87bf56c28b6dd50b72c78986c (patch) | |
tree | f577780c5431059b334ae019ec8e3e8c6a652797 | |
parent | 23f4984483623cf8621246004228f08fcabf51e4 (diff) |
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: [<ffffffffc023c948>] nd_attach_ndns+0x178/0x1b0 [libnvdimm]
but task is already holding lock:
(&nvdimm_bus->reconfig_mutex){+.+.+.}, at: [<ffffffffc022e0b1>] 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: <stable@vger.kernel.org>
Fixes: 8c2f7e8658df ("libnvdimm: infrastructure for btt devices")
Reported-by: Yi Zhang <yizhan@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
-rw-r--r-- | drivers/nvdimm/btt_devs.c | 2 | ||||
-rw-r--r-- | drivers/nvdimm/claim.c | 23 | ||||
-rw-r--r-- | drivers/nvdimm/dax_devs.c | 2 | ||||
-rw-r--r-- | 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) | |||
314 | if (rc < 0) { | 314 | if (rc < 0) { |
315 | struct nd_btt *nd_btt = to_nd_btt(btt_dev); | 315 | struct nd_btt *nd_btt = to_nd_btt(btt_dev); |
316 | 316 | ||
317 | __nd_detach_ndns(btt_dev, &nd_btt->ndns); | 317 | nd_detach_ndns(btt_dev, &nd_btt->ndns); |
318 | put_device(btt_dev); | 318 | put_device(btt_dev); |
319 | } | 319 | } |
320 | 320 | ||
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 @@ | |||
21 | void __nd_detach_ndns(struct device *dev, struct nd_namespace_common **_ndns) | 21 | void __nd_detach_ndns(struct device *dev, struct nd_namespace_common **_ndns) |
22 | { | 22 | { |
23 | struct nd_namespace_common *ndns = *_ndns; | 23 | struct nd_namespace_common *ndns = *_ndns; |
24 | struct nvdimm_bus *nvdimm_bus; | ||
24 | 25 | ||
25 | lockdep_assert_held(&ndns->dev.mutex); | 26 | if (!ndns) |
27 | return; | ||
28 | |||
29 | nvdimm_bus = walk_to_nvdimm_bus(&ndns->dev); | ||
30 | lockdep_assert_held(&nvdimm_bus->reconfig_mutex); | ||
26 | dev_WARN_ONCE(dev, ndns->claim != dev, "%s: invalid claim\n", __func__); | 31 | dev_WARN_ONCE(dev, ndns->claim != dev, "%s: invalid claim\n", __func__); |
27 | ndns->claim = NULL; | 32 | ndns->claim = NULL; |
28 | *_ndns = NULL; | 33 | *_ndns = NULL; |
@@ -37,18 +42,20 @@ void nd_detach_ndns(struct device *dev, | |||
37 | if (!ndns) | 42 | if (!ndns) |
38 | return; | 43 | return; |
39 | get_device(&ndns->dev); | 44 | get_device(&ndns->dev); |
40 | device_lock(&ndns->dev); | 45 | nvdimm_bus_lock(&ndns->dev); |
41 | __nd_detach_ndns(dev, _ndns); | 46 | __nd_detach_ndns(dev, _ndns); |
42 | device_unlock(&ndns->dev); | 47 | nvdimm_bus_unlock(&ndns->dev); |
43 | put_device(&ndns->dev); | 48 | put_device(&ndns->dev); |
44 | } | 49 | } |
45 | 50 | ||
46 | bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, | 51 | bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, |
47 | struct nd_namespace_common **_ndns) | 52 | struct nd_namespace_common **_ndns) |
48 | { | 53 | { |
54 | struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(&attach->dev); | ||
55 | |||
49 | if (attach->claim) | 56 | if (attach->claim) |
50 | return false; | 57 | return false; |
51 | lockdep_assert_held(&attach->dev.mutex); | 58 | lockdep_assert_held(&nvdimm_bus->reconfig_mutex); |
52 | dev_WARN_ONCE(dev, *_ndns, "%s: invalid claim\n", __func__); | 59 | dev_WARN_ONCE(dev, *_ndns, "%s: invalid claim\n", __func__); |
53 | attach->claim = dev; | 60 | attach->claim = dev; |
54 | *_ndns = attach; | 61 | *_ndns = attach; |
@@ -61,9 +68,9 @@ bool nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach, | |||
61 | { | 68 | { |
62 | bool claimed; | 69 | bool claimed; |
63 | 70 | ||
64 | device_lock(&attach->dev); | 71 | nvdimm_bus_lock(&attach->dev); |
65 | claimed = __nd_attach_ndns(dev, attach, _ndns); | 72 | claimed = __nd_attach_ndns(dev, attach, _ndns); |
66 | device_unlock(&attach->dev); | 73 | nvdimm_bus_unlock(&attach->dev); |
67 | return claimed; | 74 | return claimed; |
68 | } | 75 | } |
69 | 76 | ||
@@ -114,7 +121,7 @@ static void nd_detach_and_reset(struct device *dev, | |||
114 | struct nd_namespace_common **_ndns) | 121 | struct nd_namespace_common **_ndns) |
115 | { | 122 | { |
116 | /* detach the namespace and destroy / reset the device */ | 123 | /* detach the namespace and destroy / reset the device */ |
117 | nd_detach_ndns(dev, _ndns); | 124 | __nd_detach_ndns(dev, _ndns); |
118 | if (is_idle(dev, *_ndns)) { | 125 | if (is_idle(dev, *_ndns)) { |
119 | nd_device_unregister(dev, ND_ASYNC); | 126 | nd_device_unregister(dev, ND_ASYNC); |
120 | } else if (is_nd_btt(dev)) { | 127 | } else if (is_nd_btt(dev)) { |
@@ -184,7 +191,7 @@ ssize_t nd_namespace_store(struct device *dev, | |||
184 | } | 191 | } |
185 | 192 | ||
186 | WARN_ON_ONCE(!is_nvdimm_bus_locked(dev)); | 193 | WARN_ON_ONCE(!is_nvdimm_bus_locked(dev)); |
187 | if (!nd_attach_ndns(dev, ndns, _ndns)) { | 194 | if (!__nd_attach_ndns(dev, ndns, _ndns)) { |
188 | dev_dbg(dev, "%s already claimed\n", | 195 | dev_dbg(dev, "%s already claimed\n", |
189 | dev_name(&ndns->dev)); | 196 | dev_name(&ndns->dev)); |
190 | len = -EBUSY; | 197 | 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) | |||
124 | dev_dbg(dev, "%s: dax: %s\n", __func__, | 124 | dev_dbg(dev, "%s: dax: %s\n", __func__, |
125 | rc == 0 ? dev_name(dax_dev) : "<none>"); | 125 | rc == 0 ? dev_name(dax_dev) : "<none>"); |
126 | if (rc < 0) { | 126 | if (rc < 0) { |
127 | __nd_detach_ndns(dax_dev, &nd_pfn->ndns); | 127 | nd_detach_ndns(dax_dev, &nd_pfn->ndns); |
128 | put_device(dax_dev); | 128 | put_device(dax_dev); |
129 | } else | 129 | } else |
130 | __nd_device_register(dax_dev); | 130 | __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) | |||
484 | dev_dbg(dev, "%s: pfn: %s\n", __func__, | 484 | dev_dbg(dev, "%s: pfn: %s\n", __func__, |
485 | rc == 0 ? dev_name(pfn_dev) : "<none>"); | 485 | rc == 0 ? dev_name(pfn_dev) : "<none>"); |
486 | if (rc < 0) { | 486 | if (rc < 0) { |
487 | __nd_detach_ndns(pfn_dev, &nd_pfn->ndns); | 487 | nd_detach_ndns(pfn_dev, &nd_pfn->ndns); |
488 | put_device(pfn_dev); | 488 | put_device(pfn_dev); |
489 | } else | 489 | } else |
490 | __nd_device_register(pfn_dev); | 490 | __nd_device_register(pfn_dev); |