Hello, Another comment on the kernel 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> Looking at the way that suspend-blocks are used in the current Android 'msm' kernel tree[1], they seem likely to cause layering violations, fragile kernel code with userspace dependencies, and code that consumes power unnecessarily. For example, in drivers/mmc/core/core.c:721, in mmc_rescan() [2], we find the following code: /* give userspace some time to react */ wake_lock_timeout(&mmc_delayed_work_wake_lock, HZ / 2); This is a layering violation. The MMC subsystem should have nothing to do with "giving userspace time to react." That is the responsibility of the Linux scheduler. This code is also intrinsically fragile and use-case dependent. What if userspace occasionally needs more than (HZ / 2) to react? If the distributor is lucky enough to catch this before the product ships, then the distributor can patch in a new magic number. But if the device makes it to the consumer, the result is an unstable device that unpredictably suspends. The above code will also waste power. Even if userspace takes less than (HZ / 2) to react, the system will still be prevented from entering a system-wide low power state for the duration of the remaining time. This is in contrast to an approach that uses the idle loop to enter a system-wide low power state. The moment that the system has no further work to do, it can start saving power. The problems with the above example are not isolated: [paul@twilight msm]$ fgrep -r wake_lock_timeout . | egrep '(^./drivers|^./arch)' ./arch/arm/mach-msm/htc_battery.c: wake_lock_timeout(&vbus_wake_lock, HZ / 2); ./arch/arm/mach-msm/smd_tty.c: wake_lock_timeout(&info->wake_lock, HZ / 2); ./arch/arm/mach-msm/smd_qmi.c: wake_lock_timeout(&ctxt->wake_lock, HZ / 2); ./drivers/rtc/alarm.c: wake_lock_timeout(&alarm_wake_lock, 5 * HZ); ./drivers/rtc/alarm.c: wake_lock_timeout(&alarm_rtc_wake_lock, 1 * HZ); ./drivers/rtc/alarm.c: wake_lock_timeout(&alarm_rtc_wake_lock, 2 * HZ); ./drivers/mmc/core/core.c: wake_lock_timeout(&mmc_delayed_work_wake_lock, HZ / 2); ./drivers/net/msm_rmnet.c: wake_lock_timeout(&p->wake_lock, HZ / 2); ./drivers/video/msm/msm_fb.c: wake_lock_timeout(&msmfb->idle_lock, HZ/4); ./drivers/video/msm/msm_fb.c: wake_lock_timeout(&msmfb->idle_lock, HZ/4); ./drivers/video/msm/msm_fb.c: wake_lock_timeout(&msmfb->idle_lock, HZ); ./drivers/input/evdev.c: wake_lock_timeout(&client->wake_lock, 5 * HZ); ./drivers/serial/msm_serial_hs.c: wake_lock_timeout(&msm_uport->rx.wake_lock, HZ / 2); It is true that the current patch series does not propose the *_timeout() interface. But that raises the question of what it will be replaced with, in situations where the problem can't simply be pushed to userspace, as it is in Arve's evdev patch 7. Some of these cases seem difficult to handle without some sort of timer-based approach. Timer-based approaches are intrinsically problematic for the reasons mentioned above. - Paul 1. git://android.git.kernel.org/kernel/msm.git - as of commit c7f8bcecb06b937c45dd0e342450a3218b286b8d, "[ARM] msm: defconfig: Set mmap_min_addr to 32k" 2. 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
