* [PATCH] ACPI / HOTPLUG: fix device->physical_node_lock deadlock
@ 2015-04-07 9:03 Xie XiuQi
2015-04-07 11:22 ` Rafael J. Wysocki
0 siblings, 1 reply; 9+ messages in thread
From: Xie XiuQi @ 2015-04-07 9:03 UTC (permalink / raw)
To: rjw, lenb; +Cc: guohanjun, hanjun.guo, linux-acpi, linux-kernel
I meet a deadlock during cpu hotplug. The code path is bellow:
Call Trace:
[<ffffffff816e373c>] dump_stack+0x19/0x1b
[<ffffffff810fd85a>] validate_chain.isra.43+0xf4a/0x1120
[<ffffffff810236c9>] ? sched_clock+0x9/0x10
[<ffffffff810ca8bd>] ? sched_clock_local+0x1d/0x80
[<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
[<ffffffff810fe846>] __lock_acquire+0x3c6/0xb70
[<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
[<ffffffff810ff7e2>] lock_acquire+0xa2/0x1f0
[<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
[<ffffffff816e7a14>] mutex_lock_nested+0x94/0x3f0
[<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
[<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
[<ffffffff810fe0fd>] ? trace_hardirqs_on+0xd/0x10
[<ffffffff813ba132>] acpi_scan_is_offline+0x2c/0xa3 --> LOCK (DEADLOCK)
[<ffffffff813fdac8>] acpi_container_offline+0x32/0x4e
[<ffffffff81469e59>] container_offline+0x19/0x20
[<ffffffff81462955>] device_offline+0x95/0xc0
[<ffffffff813b9e53>] acpi_bus_offline+0xbc/0x126 --> LOCK
[<ffffffff813bb83d>] acpi_device_hotplug+0x236/0x46b
[<ffffffff813b4c75>] acpi_hotplug_work_fn+0x1e/0x29
[<ffffffff810a6c10>] process_one_work+0x220/0x710
[<ffffffff810a6ba4>] ? process_one_work+0x1b4/0x710
[<ffffffff810a721b>] worker_thread+0x11b/0x3a0
[<ffffffff810a7100>] ? process_one_work+0x710/0x710
[<ffffffff810b061d>] kthread+0xed/0x100
[<ffffffff810b0530>] ? insert_kthread_work+0x80/0x80
[<ffffffff816f663c>] ret_from_fork+0x7c/0xb0
[<ffffffff810b0530>] ? insert_kthread_work+0x80/0x80
This deadlock was introduced by commit caa73ea
("ACPI / hotplug / driver core: Handle containers in a special way").
In this patch, we just introduced a lockless version __acpi_scan_is_offline()
for acpi_container_offline(), to avoid this deadlock.
Cc: <stable@vger.kernel.org> # v3.14+
Signed-off-by: Xie XiuQi <xiexiuqi@huawei.com>
---
drivers/acpi/container.c | 2 +-
drivers/acpi/internal.h | 1 +
drivers/acpi/scan.c | 15 ++++++++++++---
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index c8ead9f..43bda3b2 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -50,7 +50,7 @@ static int acpi_container_offline(struct container_dev *cdev)
/* Check all of the dependent devices' physical companions. */
list_for_each_entry(child, &adev->children, node)
- if (!acpi_scan_is_offline(child, false))
+ if (!__acpi_scan_is_offline(child, false))
return -EBUSY;
return 0;
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 56b321a..3b7a07b 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -80,6 +80,7 @@ void acpi_apd_init(void);
acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
bool acpi_queue_hotplug_work(struct work_struct *work);
void acpi_device_hotplug(struct acpi_device *adev, u32 src);
+bool __acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
/* --------------------------------------------------------------------------
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index bbca783..ea55a9a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -293,13 +293,12 @@ acpi_device_modalias_show(struct device *dev, struct device_attribute *attr, cha
}
static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
-bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
+/* Must be called under physical_node_lock. */
+bool __acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
{
struct acpi_device_physical_node *pn;
bool offline = true;
- mutex_lock(&adev->physical_node_lock);
-
list_for_each_entry(pn, &adev->physical_node_list, node)
if (device_supports_offline(pn->dev) && !pn->dev->offline) {
if (uevent)
@@ -309,7 +308,17 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
break;
}
+ return offline;
+}
+
+bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
+{
+ bool offline = true;
+
+ mutex_lock(&adev->physical_node_lock);
+ offline = __acpi_scan_is_offline(adev, uevent);
mutex_unlock(&adev->physical_node_lock);
+
return offline;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ACPI / HOTPLUG: fix device->physical_node_lock deadlock
2015-04-07 9:03 [PATCH] ACPI / HOTPLUG: fix device->physical_node_lock deadlock Xie XiuQi
@ 2015-04-07 11:22 ` Rafael J. Wysocki
2015-04-07 11:50 ` Rafael J. Wysocki
0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-07 11:22 UTC (permalink / raw)
To: Xie XiuQi; +Cc: lenb, guohanjun, hanjun.guo, linux-acpi, linux-kernel
On Tuesday, April 07, 2015 05:03:12 PM Xie XiuQi wrote:
> I meet a deadlock during cpu hotplug. The code path is bellow:
>
> Call Trace:
> [<ffffffff816e373c>] dump_stack+0x19/0x1b
> [<ffffffff810fd85a>] validate_chain.isra.43+0xf4a/0x1120
> [<ffffffff810236c9>] ? sched_clock+0x9/0x10
> [<ffffffff810ca8bd>] ? sched_clock_local+0x1d/0x80
> [<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
> [<ffffffff810fe846>] __lock_acquire+0x3c6/0xb70
> [<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
> [<ffffffff810ff7e2>] lock_acquire+0xa2/0x1f0
> [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> [<ffffffff816e7a14>] mutex_lock_nested+0x94/0x3f0
> [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> [<ffffffff810fe0fd>] ? trace_hardirqs_on+0xd/0x10
> [<ffffffff813ba132>] acpi_scan_is_offline+0x2c/0xa3 --> LOCK (DEADLOCK)
Is it the same device, actually? acpi_container_offline() walks the *children*
of the container while acpi_bus_offline() locks the container itself.
Is it not the case?
> [<ffffffff813fdac8>] acpi_container_offline+0x32/0x4e
> [<ffffffff81469e59>] container_offline+0x19/0x20
> [<ffffffff81462955>] device_offline+0x95/0xc0
> [<ffffffff813b9e53>] acpi_bus_offline+0xbc/0x126 --> LOCK
> [<ffffffff813bb83d>] acpi_device_hotplug+0x236/0x46b
> [<ffffffff813b4c75>] acpi_hotplug_work_fn+0x1e/0x29
> [<ffffffff810a6c10>] process_one_work+0x220/0x710
> [<ffffffff810a6ba4>] ? process_one_work+0x1b4/0x710
> [<ffffffff810a721b>] worker_thread+0x11b/0x3a0
> [<ffffffff810a7100>] ? process_one_work+0x710/0x710
> [<ffffffff810b061d>] kthread+0xed/0x100
> [<ffffffff810b0530>] ? insert_kthread_work+0x80/0x80
> [<ffffffff816f663c>] ret_from_fork+0x7c/0xb0
> [<ffffffff810b0530>] ? insert_kthread_work+0x80/0x80
>
> This deadlock was introduced by commit caa73ea
> ("ACPI / hotplug / driver core: Handle containers in a special way").
>
> In this patch, we just introduced a lockless version __acpi_scan_is_offline()
> for acpi_container_offline(), to avoid this deadlock.
So why is this a correct approach? Why can acpi_container_offline() suddenly
call __acpi_scan_is_offline() without the lock?
> Cc: <stable@vger.kernel.org> # v3.14+
> Signed-off-by: Xie XiuQi <xiexiuqi@huawei.com>
> ---
> drivers/acpi/container.c | 2 +-
> drivers/acpi/internal.h | 1 +
> drivers/acpi/scan.c | 15 ++++++++++++---
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> index c8ead9f..43bda3b2 100644
> --- a/drivers/acpi/container.c
> +++ b/drivers/acpi/container.c
> @@ -50,7 +50,7 @@ static int acpi_container_offline(struct container_dev *cdev)
>
> /* Check all of the dependent devices' physical companions. */
> list_for_each_entry(child, &adev->children, node)
> - if (!acpi_scan_is_offline(child, false))
> + if (!__acpi_scan_is_offline(child, false))
> return -EBUSY;
>
> return 0;
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 56b321a..3b7a07b 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -80,6 +80,7 @@ void acpi_apd_init(void);
> acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
> bool acpi_queue_hotplug_work(struct work_struct *work);
> void acpi_device_hotplug(struct acpi_device *adev, u32 src);
> +bool __acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
> bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
>
> /* --------------------------------------------------------------------------
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index bbca783..ea55a9a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -293,13 +293,12 @@ acpi_device_modalias_show(struct device *dev, struct device_attribute *attr, cha
> }
> static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
>
> -bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
> +/* Must be called under physical_node_lock. */
> +bool __acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
> {
> struct acpi_device_physical_node *pn;
> bool offline = true;
>
> - mutex_lock(&adev->physical_node_lock);
> -
> list_for_each_entry(pn, &adev->physical_node_list, node)
> if (device_supports_offline(pn->dev) && !pn->dev->offline) {
> if (uevent)
> @@ -309,7 +308,17 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
> break;
> }
>
> + return offline;
> +}
> +
> +bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
> +{
> + bool offline = true;
> +
> + mutex_lock(&adev->physical_node_lock);
> + offline = __acpi_scan_is_offline(adev, uevent);
> mutex_unlock(&adev->physical_node_lock);
> +
> return offline;
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ACPI / HOTPLUG: fix device->physical_node_lock deadlock
2015-04-07 11:22 ` Rafael J. Wysocki
@ 2015-04-07 11:50 ` Rafael J. Wysocki
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-07 11:50 UTC (permalink / raw)
To: Xie XiuQi; +Cc: lenb, guohanjun, hanjun.guo, linux-acpi, linux-kernel
On Tuesday, April 07, 2015 01:22:52 PM Rafael J. Wysocki wrote:
> On Tuesday, April 07, 2015 05:03:12 PM Xie XiuQi wrote:
> > I meet a deadlock during cpu hotplug. The code path is bellow:
> >
> > Call Trace:
> > [<ffffffff816e373c>] dump_stack+0x19/0x1b
> > [<ffffffff810fd85a>] validate_chain.isra.43+0xf4a/0x1120
> > [<ffffffff810236c9>] ? sched_clock+0x9/0x10
> > [<ffffffff810ca8bd>] ? sched_clock_local+0x1d/0x80
> > [<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
> > [<ffffffff810fe846>] __lock_acquire+0x3c6/0xb70
> > [<ffffffff810caa88>] ? sched_clock_cpu+0xa8/0x100
> > [<ffffffff810ff7e2>] lock_acquire+0xa2/0x1f0
> > [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> > [<ffffffff816e7a14>] mutex_lock_nested+0x94/0x3f0
> > [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> > [<ffffffff813ba132>] ? acpi_scan_is_offline+0x2c/0xa3
> > [<ffffffff810fe0fd>] ? trace_hardirqs_on+0xd/0x10
> > [<ffffffff813ba132>] acpi_scan_is_offline+0x2c/0xa3 --> LOCK (DEADLOCK)
>
> Is it the same device, actually? acpi_container_offline() walks the *children*
> of the container while acpi_bus_offline() locks the container itself.
So the patch below should make the splat go away too if I'm not mistaken.
---
drivers/acpi/scan.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -298,7 +298,11 @@ bool acpi_scan_is_offline(struct acpi_de
struct acpi_device_physical_node *pn;
bool offline = true;
- mutex_lock(&adev->physical_node_lock);
+ /*
+ * acpi_container_offline() calls this for all of the container's
+ * children under the container's physical_node_lock lock.
+ */
+ mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING);
list_for_each_entry(pn, &adev->physical_node_list, node)
if (device_supports_offline(pn->dev) && !pn->dev->offline) {
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-07 11:50 ` Rafael J. Wysocki
@ 2015-04-10 23:31 ` Rafael J. Wysocki
2015-04-13 1:28 ` Xie XiuQi
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-10 23:31 UTC (permalink / raw)
To: linux-acpi; +Cc: Xie XiuQi, guohanjun, hanjun.guo, linux-kernel, Toshi Kani
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
acpi_scan_is_offline() may be called under the physical_node_lock
of the given device object's parent, so prevent lockdep from
complaining about that by annotating that instance with
SINGLE_DEPTH_NESTING.
Reported-by: Xie XiuQi <xiexiuqi@huawei.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -298,7 +298,11 @@ bool acpi_scan_is_offline(struct acpi_de
struct acpi_device_physical_node *pn;
bool offline = true;
- mutex_lock(&adev->physical_node_lock);
+ /*
+ * acpi_container_offline() calls this for all of the container's
+ * children under the container's physical_node_lock lock.
+ */
+ mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING);
list_for_each_entry(pn, &adev->physical_node_list, node)
if (device_supports_offline(pn->dev) && !pn->dev->offline) {
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
@ 2015-04-13 1:28 ` Xie XiuQi
2015-04-13 8:27 ` Hanjun Guo
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Xie XiuQi @ 2015-04-13 1:28 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi
Cc: guohanjun, hanjun.guo, linux-kernel, Toshi Kani
On 2015/4/11 7:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> acpi_scan_is_offline() may be called under the physical_node_lock
> of the given device object's parent, so prevent lockdep from
> complaining about that by annotating that instance with
> SINGLE_DEPTH_NESTING.
>
> Reported-by: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Thank you for your patch, I'll test soon.
Thanks,
Xie XiuQi
> ---
> drivers/acpi/scan.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -298,7 +298,11 @@ bool acpi_scan_is_offline(struct acpi_de
> struct acpi_device_physical_node *pn;
> bool offline = true;
>
> - mutex_lock(&adev->physical_node_lock);
> + /*
> + * acpi_container_offline() calls this for all of the container's
> + * children under the container's physical_node_lock lock.
> + */
> + mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING);
>
> list_for_each_entry(pn, &adev->physical_node_list, node)
> if (device_supports_offline(pn->dev) && !pn->dev->offline) {
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
2015-04-13 1:28 ` Xie XiuQi
@ 2015-04-13 8:27 ` Hanjun Guo
2015-04-13 13:48 ` Rafael J. Wysocki
2015-04-16 19:01 ` Toshi Kani
2015-04-17 7:19 ` Xie XiuQi
3 siblings, 1 reply; 9+ messages in thread
From: Hanjun Guo @ 2015-04-13 8:27 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi
Cc: Xie XiuQi, guohanjun, linux-kernel, Toshi Kani
On 2015年04月11日 07:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> acpi_scan_is_offline() may be called under the physical_node_lock
> of the given device object's parent, so prevent lockdep from
> complaining about that by annotating that instance with
> SINGLE_DEPTH_NESTING.
I think this is trigged by setting acpi_force_hot_remove to 1,
in acpi_scan_hot_remove():
if (device->handler && device->handler->hotplug.demand_offline
&& !acpi_force_hot_remove) {
if (!acpi_scan_is_offline(device, true))
return -EBUSY;
} else {
int error = acpi_scan_try_to_offline(device);
if (error)
return error;
}
then the container device will be removed by acpi_scan_try_to_offline(),
let's wait for Xiuqi's test result.
Thanks
Hanjun
>
> Reported-by: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/scan.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -298,7 +298,11 @@ bool acpi_scan_is_offline(struct acpi_de
> struct acpi_device_physical_node *pn;
> bool offline = true;
>
> - mutex_lock(&adev->physical_node_lock);
> + /*
> + * acpi_container_offline() calls this for all of the container's
> + * children under the container's physical_node_lock lock.
> + */
> + mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING);
>
> list_for_each_entry(pn, &adev->physical_node_list, node)
> if (device_supports_offline(pn->dev) && !pn->dev->offline) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-13 8:27 ` Hanjun Guo
@ 2015-04-13 13:48 ` Rafael J. Wysocki
0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-04-13 13:48 UTC (permalink / raw)
To: Hanjun Guo; +Cc: linux-acpi, Xie XiuQi, guohanjun, linux-kernel, Toshi Kani
On Monday, April 13, 2015 04:27:16 PM Hanjun Guo wrote:
> On 2015年04月11日 07:31, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > acpi_scan_is_offline() may be called under the physical_node_lock
> > of the given device object's parent, so prevent lockdep from
> > complaining about that by annotating that instance with
> > SINGLE_DEPTH_NESTING.
>
> I think this is trigged by setting acpi_force_hot_remove to 1,
> in acpi_scan_hot_remove():
>
> if (device->handler && device->handler->hotplug.demand_offline
> && !acpi_force_hot_remove) {
> if (!acpi_scan_is_offline(device, true))
> return -EBUSY;
> } else {
> int error = acpi_scan_try_to_offline(device);
> if (error)
> return error;
> }
>
> then the container device will be removed by acpi_scan_try_to_offline(),
> let's wait for Xiuqi's test result.
I'm not sure what you mean. demand_offline is 'true' for containers, so
acpi_force_hot_remove doesn't matter here.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
2015-04-13 1:28 ` Xie XiuQi
2015-04-13 8:27 ` Hanjun Guo
@ 2015-04-16 19:01 ` Toshi Kani
2015-04-17 7:19 ` Xie XiuQi
3 siblings, 0 replies; 9+ messages in thread
From: Toshi Kani @ 2015-04-16 19:01 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-acpi, Xie XiuQi, guohanjun, hanjun.guo, linux-kernel
On Sat, 2015-04-11 at 01:31 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> acpi_scan_is_offline() may be called under the physical_node_lock
> of the given device object's parent, so prevent lockdep from
> complaining about that by annotating that instance with
> SINGLE_DEPTH_NESTING.
>
> Reported-by: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Agreed that the issue is likely with the lock-class. Assuming Xie's
test goes well (we will need to wait for that :),
Reviewed-by: Toshi Kani <toshi.kani@hp.com>
Thanks,
-Toshi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline()
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
` (2 preceding siblings ...)
2015-04-16 19:01 ` Toshi Kani
@ 2015-04-17 7:19 ` Xie XiuQi
3 siblings, 0 replies; 9+ messages in thread
From: Xie XiuQi @ 2015-04-17 7:19 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-acpi
Cc: guohanjun, hanjun.guo, linux-kernel, Toshi Kani
On 2015/4/11 7:31, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> acpi_scan_is_offline() may be called under the physical_node_lock
> of the given device object's parent, so prevent lockdep from
> complaining about that by annotating that instance with
> SINGLE_DEPTH_NESTING.
>
> Reported-by: Xie XiuQi <xiexiuqi@huawei.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Xie XiuQi <xiexiuqi@huawei.com>
Thanks,
Xie XiuQi
> ---
> drivers/acpi/scan.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -298,7 +298,11 @@ bool acpi_scan_is_offline(struct acpi_de
> struct acpi_device_physical_node *pn;
> bool offline = true;
>
> - mutex_lock(&adev->physical_node_lock);
> + /*
> + * acpi_container_offline() calls this for all of the container's
> + * children under the container's physical_node_lock lock.
> + */
> + mutex_lock_nested(&adev->physical_node_lock, SINGLE_DEPTH_NESTING);
>
> list_for_each_entry(pn, &adev->physical_node_list, node)
> if (device_supports_offline(pn->dev) && !pn->dev->offline) {
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-17 7:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-07 9:03 [PATCH] ACPI / HOTPLUG: fix device->physical_node_lock deadlock Xie XiuQi
2015-04-07 11:22 ` Rafael J. Wysocki
2015-04-07 11:50 ` Rafael J. Wysocki
2015-04-10 23:31 ` [PATCH] ACPI / scan: Annotate physical_node_lock in acpi_scan_is_offline() Rafael J. Wysocki
2015-04-13 1:28 ` Xie XiuQi
2015-04-13 8:27 ` Hanjun Guo
2015-04-13 13:48 ` Rafael J. Wysocki
2015-04-16 19:01 ` Toshi Kani
2015-04-17 7:19 ` Xie XiuQi
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).