Hello,
some comments on the kernel-level suspend blocker API.
On Thu, 13 May 2010, Arve Hjønnevåg wrote:
> Adds /sys/power/policy that selects the behaviour of /sys/power/state.
> After setting the policy to opportunistic, writes to /sys/power/state
> become non-blocking requests that specify which suspend state to enter
> when no suspend blockers are active. A special state, "on", stops the
> process by activating the "main" suspend blocker.
>
> Signed-off-by: Arve Hjønnevåg <arve@xxxxxxxxxxx>
Another problem with the suspend-block kernel API is that its primary
use-case[1] is to work around the brokenness of patch 1's opportunistic
suspend governor. The governor is broken because it intentionally ignores
timers and the scheduler. To work around problems caused by this
decision, suspend-blocks must be spread throughout the kernel (and
userspace) to restore the previous behavior of the system. But if patch
1's opportunistic suspend governor were fixed, most of the need for a
suspend blockers would disappear.
[ This E-mail expands on some earlier comments[2] and is also related
to other opportunistic suspend comments[3]. ]
The suspend-block patches require already-working code to be patched to
keep it working. This is a backwards approach. Instead, broken code
should be patched to fix the brokenness. But the former approach is
exactly what the suspend block API is intended to do. Patches 7 and 8 in
this series are two clear cases. Patch 7 adds an ioctl to the input
subsystem that will cause the system to stay running as long as there is
an event in the event queue. Patch 8 adds a suspend-block call to the
power_supply code that causes the system to stay running as long as
power_supply's changed_work workqueue has work to do. Neither of these
patches should be needed, since the default expectation of the mainline
Linux kernel is that the system should run when there is work to be done.
This longstanding expectation should be preserved, not broken and then
patched around. But since patch 1 violates that expectation, there is no
choice but to add patches 7 and 8 -- and ultimately many more patches --
to get a working system. Once the opportunistic suspend governor in patch
1 is introduced and enabled, the kernel will suspend the system
immediately, even if the system has work to do. The only way to prevent
that is to add suspend-blocks.
Many suspend blocks will be needed throughout the kernel to keep things
working. One might think that patches 7 and 8 are the only changes needed
to get a working Android-style opportunistic suspend system. It turns out
that many more changes are needed. This can be seen by looking at
currently shipping Android kernels, such as the current current Android
'msm' kernel tree[4]. Suspend-blocker usage is spread through many drivers
and subsystems:
[paul@twilight msm]$ fgrep -r wake_lock . | egrep '(^./drivers|^./arch)' |
cut -f1 -d: | sort -u
./arch/arm/mach-msm/htc_battery.c
./arch/arm/mach-msm/pm.c
./arch/arm/mach-msm/qdsp5/adsp.c
./arch/arm/mach-msm/qdsp5/audio_out.c
./arch/arm/mach-msm/smd_qmi.c
./arch/arm/mach-msm/smd_rpcrouter.c
./arch/arm/mach-msm/smd_rpcrouter.h
./arch/arm/mach-msm/smd_rpcrouter_servers.c
./arch/arm/mach-msm/smd_tty.c
./drivers/i2c/chips/mt9t013.c
./drivers/input/evdev.c
./drivers/input/misc/gpio_input.c
./drivers/input/misc/gpio_matrix.c
./drivers/mmc/core/core.c
./drivers/net/msm_rmnet.c
./drivers/rtc/alarm.c
./drivers/serial/msm_serial_hs.c
./drivers/usb/function/mass_storage.c
./drivers/usb/gadget/f_mass_storage.c
./drivers/video/msm/mddi.c
./drivers/video/msm/msm_fb.c
I guess this is something like what kernel developers should eventually
expect to see if suspend blockers are merged.
There is a better way to make the system work as expected: these patch 1
shouldn't break general timer and scheduler assumptions. If patch 1's
governor is fixed, most of these calls will be unnecessary. The apparent
motivation is to prevent "untrusted" userspace processes from preventing
the system from entering a low-power state. I agree that this is a
problem. But it should be fixed by modifying the specific Linux code that
determines when the idle loop is entered and when the next timer wakeup
should occur, on a per-process basis. Such targeted changes would ensure
that the rest of the kernel would continue to execute as expected.
regards,
- Paul
1. Arve Hjønnevåg E-mail to the linux-pm mailing list, dated Mon, 3
May 2010 17:09:22 -0700:
http://www.spinics.net/lists/linux-pm/msg18432.html
2. Paul Walmsley E-mail to the linux-pm mailing list, dated Wed, 12
May 2010 21:35:30 -0600: http://lkml.org/lkml/2010/5/12/528
3. Paul Walmsley E-mail to the linux-pm mailing list, dated Thu, 13
May 2010 13:01:33 -0600:
http://permalink.gmane.org/gmane.linux.power-management.general/18592
4. git://android.git.kernel.org/kernel/msm.git - as of commit
c7f8bcecb06b937c45dd0e342450a3218b286b8d, "[ARM] msm: defconfig:
Set mmap_min_addr to 32k"
5. http://android.git.kernel.org/?p=kernel/msm.git;a=blob;f=drivers/mmc/core/core.c;h=22f6ddad85c92f34c55035982049bb03b5abfc74;hb=c7f8bcecb06b937c45dd0e342450a3218b286b8d#l721
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
