[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200711210232.30575.rjw@sisk.pl>
Date: Wed, 21 Nov 2007 02:32:30 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Franck Bui-Huu <vagabon.xyz@...il.com>
Cc: linux-pm@...ts.linux-foundation.org,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: apm emulation driver broken ?
On Monday, 19 of November 2007, Franck Bui-Huu wrote:
> Rafael J. Wysocki wrote:
> > On Sunday, 18 of November 2007, Franck Bui-Huu wrote:
> >> Rafael J. Wysocki wrote:
> >> See the call to wait_even() made by apm_ioctl(). If any processes
> >> run this, it will prevent the system to suspend...
> >
> > True, but does it actually happen in practice?
> >
>
> when several processes are waiting for a suspend event.
>
> >
> > At this point the second branch of the "if (as->suspend_state == SUSPEND_READ)"
> > can be fixed by replacing wait_event_interruptible() with
> > wait_event_freezable(),
>
> yes
>
> > but the fix for the first branch depends on whether or
> > not the wait_event() is really necessary.
>
> As I said I don't know. It's probably time to put some people
> on CC but don't know who though.
OK, never mind. I think the patch below is the right fix.
---
From: Rafael J. Wysocki <rjw@...k.pl>
The APM emulation is currently broken as a result of commit
831441862956fffa17b9801db37e6ea1650b0f69
"Freezer: make kernel threads nonfreezable by default"
that removed the PF_NOFREEZE annotations from apm_ioctl() without adding
the appropriate freezer hooks. Fix it and remove the unnecessary variable flags
from apm_ioctl().
Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
---
drivers/char/apm-emulation.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
Index: linux-2.6/drivers/char/apm-emulation.c
===================================================================
--- linux-2.6.orig/drivers/char/apm-emulation.c
+++ linux-2.6/drivers/char/apm-emulation.c
@@ -295,7 +295,6 @@ static int
apm_ioctl(struct inode * inode, struct file *filp, u_int cmd, u_long arg)
{
struct apm_user *as = filp->private_data;
- unsigned long flags;
int err = -EINVAL;
if (!as->suser || !as->writer)
@@ -331,10 +330,16 @@ apm_ioctl(struct inode * inode, struct f
* Wait for the suspend/resume to complete. If there
* are pending acknowledges, we wait here for them.
*/
- flags = current->flags;
+ freezer_do_not_count();
wait_event(apm_suspend_waitqueue,
as->suspend_state == SUSPEND_DONE);
+
+ /*
+ * Since we are waiting until the suspend is done, the
+ * try_to_freeze() in freezer_count() will not trigger
+ */
+ freezer_count();
} else {
as->suspend_state = SUSPEND_WAIT;
mutex_unlock(&state_lock);
@@ -362,14 +367,10 @@ apm_ioctl(struct inode * inode, struct f
* Wait for the suspend/resume to complete. If there
* are pending acknowledges, we wait here for them.
*/
- flags = current->flags;
-
- wait_event_interruptible(apm_suspend_waitqueue,
+ wait_event_freezable(apm_suspend_waitqueue,
as->suspend_state == SUSPEND_DONE);
}
- current->flags = flags;
-
mutex_lock(&state_lock);
err = as->suspend_result;
as->suspend_state = SUSPEND_NONE;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists