Subject: [PATCH] APM: fix deadlock in APM_IOC_SUSPEND ioctl
Subsystems (2)
I found the Xorg server on my ARM device stuck in the 'msleep()' loop in apm_ioctl.
I suspect it had attempted suspend immediately after resuming and lost a race.
During that msleep(10);, a new suspend cycle must have started and changed ->suspend_state to SUSPEND_PENDING, so it was never seen to be SUSPEND_DONE and the loop could never exited. It would have moved on to SUSPEND_ACKTO but never been able to reach SUSPEND_DONE.
So change the loop to only run while SUSPEND_ACKED rather than until SUSPEND_DONE. This is much safer.
drivers/char/apm-emulation.c +1 -1
@@ -302,7 +302,7 @@ apm_ioctl(struct file *filp, u_int cmd, u_long arg)* anything critical, chill a bit on each iteration. */ while (wait_event_freezable (apm_suspend_waitqueue, - as->suspend_state ==SUSPEND_DONE )) + as->suspend_state !=SUSPEND_ACKED ))msleep (10 );break ;case SUSPEND_ACKTO :
Jiri Kosina
AppliedQuoted 22 lines · NeilBrown
I found the Xorg server on my ARM device stuck in the '
msleep()' loop inapm_ioctl.I suspect it had attempted suspend immediately after resuming and lost a race.
During that msleep(10);, a new suspend cycle must have started and changed ->suspend_state toSUSPEND_PENDING, so it was never seen to beSUSPEND_DONEand the loop could never exited. It would have moved on toSUSPEND_ACKTObut never been able to reachSUSPEND_DONE.So change the loop to only run while
SUSPEND_ACKEDrather than untilSUSPEND_DONE. This is much safer.Signed-off-by NeilBrown <[email protected]>drivers/char/apm-emulation.c +0
-0
drivers/char/apm-emulation.c apm_ioctl(struct file *filp, u_int cmd, u_long arg) · line 308
Quoted 8 lines
drivers/char/apm-emulation.c +1
-1@@ -302,7 +302,7 @@ apm_ioctl(struct file *filp, u_int cmd, u_long arg)* anything critical, chill a bit on each iteration. */ while (wait_event_freezable (apm_suspend_waitqueue, - as->suspend_state ==SUSPEND_DONE )) + as->suspend_state !=SUSPEND_ACKED ))msleep (10 );break ;case SUSPEND_ACKTO :
Good catch Neil! I have missed this race when we were moving away from freezer_*_count() to wait_event_freezable() here.
Applied, thanks.
Signature · 2 lines
Jiri Kosina SUSE Labs