2006-02-18 02:03:39

by Patrick Mochel

[permalink] [raw]
Subject: [PATCH 1/5] [pm] Fix locking of device suspend/resume functions


This patch removes the unneeded down()/up() calls from
suspend_device() and resume_device(). Those functions
are already called under the dpm_sem, making this code
unconditionally deadlock in SMP kernels.

Signed-off-by: Patrick Mochel <[email protected]>

---

drivers/base/power/resume.c | 3 ---
drivers/base/power/suspend.c | 2 --
2 files changed, 0 insertions(+), 5 deletions(-)

applies-to: 55ce8c6305fc70b1b544ce7365abd6054e9b5f61
1bf4a2adaa1588c3d68038f56e1f7c9cb96a3af9
diff --git a/drivers/base/power/resume.c b/drivers/base/power/resume.c
index 317edbf..478e116 100644
--- a/drivers/base/power/resume.c
+++ b/drivers/base/power/resume.c
@@ -23,7 +23,6 @@ int resume_device(struct device * dev)
{
int error = 0;

- down(&dev->sem);
if (dev->power.pm_parent
&& dev->power.pm_parent->power.power_state.event) {
dev_err(dev, "PM: resume from %d, parent %s still %d\n",
@@ -35,12 +34,10 @@ int resume_device(struct device * dev)
dev_dbg(dev,"resuming\n");
error = dev->bus->resume(dev);
}
- up(&dev->sem);
return error;
}


-
void dpm_resume(void)
{
down(&dpm_list_sem);
diff --git a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
index 8660779..a59158c 100644
--- a/drivers/base/power/suspend.c
+++ b/drivers/base/power/suspend.c
@@ -38,7 +38,6 @@ int suspend_device(struct device * dev,
{
int error = 0;

- down(&dev->sem);
if (dev->power.power_state.event) {
dev_dbg(dev, "PM: suspend %d-->%d\n",
dev->power.power_state.event, state.event);
@@ -58,7 +57,6 @@ int suspend_device(struct device * dev,
dev_dbg(dev, "suspending\n");
error = dev->bus->suspend(dev, state);
}
- up(&dev->sem);
return error;
}

---
0.99.9.GIT


2006-02-18 05:01:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/5] [pm] Fix locking of device suspend/resume functions

Patrick Mochel <[email protected]> wrote:
>
> This patch removes the unneeded down()/up() calls from
> suspend_device() and resume_device(). Those functions
> are already called under the dpm_sem, making this code
> unconditionally deadlock in SMP kernels.

I've seen no reports of such deadlocks. And I was testing swsusp on a
4-way this week?

2006-02-18 05:50:57

by Patrick Mochel

[permalink] [raw]
Subject: Re: [PATCH 1/5] [pm] Fix locking of device suspend/resume functions


On Fri, 17 Feb 2006, Andrew Morton wrote:

> Patrick Mochel <[email protected]> wrote:
> >
> > This patch removes the unneeded down()/up() calls from
> > suspend_device() and resume_device(). Those functions
> > are already called under the dpm_sem, making this code
> > unconditionally deadlock in SMP kernels.
>
> I've seen no reports of such deadlocks. And I was testing swsusp on a
> 4-way this week?

Yup, I misparsed the code. Please disregard this patch.

Thanks,


Pat

2006-02-18 09:22:22

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 1/5] [pm] Fix locking of device suspend/resume functions

On Fri, 2006-02-17 at 18:03 -0800, Patrick Mochel wrote:
> This patch removes the unneeded down()/up() calls from
> suspend_device() and resume_device(). Those functions
> are already called under the dpm_sem, making this code
> unconditionally deadlock in SMP kernels.

this wants to be a mutex as well ;)