Not logged in
Log in now
Create an account
Subscribe to LWN
LWN.net Weekly Edition for December 5, 2013
Deadline scheduling: coming soon?
LWN.net Weekly Edition for November 27, 2013
ACPI for ARM?
LWN.net Weekly Edition for November 21, 2013
Posted Dec 22, 2011 23:46 UTC (Thu) by brendan_wright (subscriber, #7376)
Posted Dec 23, 2011 0:08 UTC (Fri) by neilbrown (subscriber, #359)
But the "kernel" doesn't need to be particularly involved in this. User-space apps can talk to user-space suspend manager and when an appropriate time to suspend arrives the user-space suspend manager can initiate a suspend.
wakelocks as a feature are perfectly fine. However they don't need to be a kernel feature.
(well .. to be accurate there does need to be a way for kernel modules such as drivers to hold-off suspend for a while, but we have that - they are called wakeup-sources. Userspace uses a separate mechanism that is more fitting for user-space).
(and to be completely fair - there is a school of thought which says that we shouldn't be using suspend at all and so wakelocks aren't fine no matter where they are implemented. Adherents of this school believe that when a tab isn't visible the animation on it should stop and then when nothing is using CPU the CPU should reduce power usage. It is hard to argue against this in terms of elegance of the design. Whether it is practical is somewhat easier to argue against).
Posted Dec 23, 2011 11:15 UTC (Fri) by brendan_wright (subscriber, #7376)
> But the "kernel" doesn't need to be particularly involved in this.
I'm not fussed about how it works, just as long as we're not still arguing about either the need for them, or the implementation details, in two years time! :-/
Posted Dec 23, 2011 4:03 UTC (Fri) by dlang (✭ supporter ✭, #313)
this greatly undermines the "this is the only way to make things work" argument.
Posted Dec 23, 2011 11:27 UTC (Fri) by brendan_wright (subscriber, #7376)
I don't think anyone here is saying they are the *only* way to "make things work". But they are a working implementation of a feature I think we need ASAP.
Posted Dec 23, 2011 0:56 UTC (Fri) by mjg59 (subscriber, #23239)
Wakelocks (or a functional equivalent) are entirely necessary for the Android power management model. Is the Android power management model the best one? I don't think so, but nobody (myself included) has successfully demonstrated otherwise so I think at this point people should either do the necessary work or stop complaining.
Posted Dec 23, 2011 1:52 UTC (Fri) by neilbrown (subscriber, #359)
When user-space hits a key the keyboard driver activates a wakeup-source. This has the effect of causing the suspend request to abort (yes, this can be done race-free). The CPU says awake, the keystroke is responded to, userspace notices that the user is doing something and arranges not to go to sleep just yet (user may still throw phone out the window, but it is probably because they hit the wrong key by mistake).
And as for "do the necessary work": Please see https://lwn.net/Articles/464144/. Just as soon as I can get my new phone suspending reliably I'll be testing this is a real-life situation.
Posted Dec 23, 2011 12:36 UTC (Fri) by mjg59 (subscriber, #23239)
Posted Dec 23, 2011 19:42 UTC (Fri) by neilbrown (subscriber, #359)
They haven't been tested heavily and there is no demonstration that they are sufficient for Androids needs so it my mind, that is the next step. I would like to see android running on a mainline kernel (which is nearly possible with what it in staging in -next) and then replace little bits so that it uses mainline-preferred functionality in place of Android-specific functionality and see how it works. Then we could move on from "you should do it like this" arguments - which never work - to "I have done it like this" arguments which tend to hold a lot more water.
Timeout-based wakeup sources are only needed when interacting with an external device (such as a human or a network switch) and in that case they are always needed. e.g. if you receive a wake-on-lan magic-packet you need to assume there will be real data soon but you cannot wait forever so you need a timeout. Similarly a key stroke is usually followed by another so stay awake for it. These sorts of things always need timeouts. You should need no more or fewer timeout-based wakeup sources than you would need equivalent timeouts with Android wakelocks.
Posted Dec 23, 2011 19:57 UTC (Fri) by mjg59 (subscriber, #23239)
You can defer the timeouts to userspace instead, but then your userspace has to be amazingly aware of how everything ties together - rather than having generic userspace, you now need userspace that knows every single wakeup event that the kernel may generate, and whether or not it needs to take a timeout on them. That's more complicated than the existing Android case.
I'm not arguing that the wakeup events code is bad or evil or does nothing to get us closer to a model where Android works, just that right now it only solves the simple cases and doesn't have a clear strategy for solving the difficult ones.
Posted Dec 23, 2011 21:08 UTC (Fri) by neilbrown (subscriber, #359)
The card-detect interrupt activates a wakeup_source which is then deactivated once the insertion is visible to user-space. I'm guessing that is just after the kobject_uevent() call which reports a 'change' or 'add'.
The user-space suspend daemon has a 'prepare' and a 'confirm' stage to requesting suspend. A wakeup_source activation after 'prepare' will cause 'confirm' to fail. Between these two stages it would do something like "udevadm settle". This would ensure that any kobject_uevent had actually been handled.
The udev handler for "sd card insertion" would fsck and mount, scan for viruses and new media files and sends a dbus signal to the UI. Then exit.
The UI would notice the signal, tell the suspend-daemon that it is busy, acknowlege the signal (thus allowing udev handler to exit), and then pop up a widget saying "new music and viruses available - which should I play?" and wait for the user to respond (or timeout).
So: no timeout needed in the kernel or at all until you get to user interaction. Also the user-space side seems entirely generic.
The kernel doesn't need to know when user-space consumes the event, only when the event is visible to user-space.
The suspend-daemon *can* know if user-space has consumed the event because it can ask every user-space subsystem "Have you consumed all your events", and the prepare/commit makes this race-free. It sounds expensive to ask everyone that question but I believe it can be done quiet efficiently. e.g. at start up the subsystem says "here is an fd - you just check it yourself, don't bother me". Then "ask everyone" becomes a single "select" system call.
If you have other examples that you think are more complicated I would love to hear them - I have little practical experience in this area and would love to benefit from the experience of others.
Posted Dec 23, 2011 21:24 UTC (Fri) by mjg59 (subscriber, #23239)
If an event occurs between confirm() being run but before the suspend is requested, you'd miss that. So the suspend needs to then fail, but that's ok if you've got a token from before you called prepare() and that token's been invalidated by another wakeup event in the intervening time. So it does sound like this could be made race-free, but it does involve adding some extra complexity to the uevent code. That's not an obviously bad idea in any case - it'd be a useful thing to have for avoiding races in the existing suspend path.
Now, how about the network case? It ought to be possible in much the same way (you'd deactivate the source when the packet has been read), but there's a pretty huge distance between an irq from a network device (where you have to activate the source just in case the packet is magic, which you don't yet know) and userspace, and it could get lost at various points in between - so you need to map every path that it could take.
This is all obviously possible, and I think your approach could certainly be made to work. But making it work well is going to involve a lot of code. Not a criticism, just an observation!
This is one of the reasons why a scheduler-based approach is more attractive in many ways - you avoid the kernel timeouts problem without having to add a huge amount of kernel complexity. It obviously does result in a bunch of additional problems on the userspace side, many of which could probably be mitigated with timer slack - but I'm obviously speaking in a purely hypothetical sense, since I haven't found the time to actually do the work necessary to make that workable. You're actually doing the work, so you get to ignore me here :)
Posted Dec 23, 2011 22:10 UTC (Fri) by neilbrown (subscriber, #359)
I don't think the uevent code would be changed.
I think for the sd card case there would be two wakeup sources - one for the MMC interface and one for the block device. So on card-detect the MMC interface activates its wakeup_source. It then inspects the card, figures out that it is a storage device, calls mmc_blk_probe, then releases its wakeup_source. mmc_blk_probe would activate its wakeup_source, do whatever it does, ultimately call add_disk (which will make the uevent visible to userspace) and then release the mmc_blk wakeup_source. So any driver in a wakeup chain needs a wakeup_source and needs to activate/deactivate as appropriate.
I don't know the network code very well so I'm guessing more than usual here, but I think there is an 'skbuf' that travels from device to socket queue. I imagine a single system-wide wakeup_source and a flag in the skbuf to say if it is active (a wakeup_source has a counter and so can be shared). Device sets it when creating an skbuf if wake-up is enabled and socket code clears it when it is added to a queue (or when the skbuf is discarded).
A network device might want a separate wakeup source if there is a gap between the interrupt happening and the packet being read from the network device.
But of course the devil is in the detail, and adding anything to fast-paths in the network code is a not an option so we would need to be very careful.
One step at a time :-)
Posted Dec 24, 2011 15:42 UTC (Sat) by ndye (subscriber, #9947)
. . . then pop up a widget saying "new music and viruses available - which should I play?" and wait . . .
First, thanks for the well-toned humor.
And in the midst of a non-childish conversation . . . made my day!
So long, Merry Christmas, and thanks for all the fish!
Posted Dec 23, 2011 23:14 UTC (Fri) by dlang (✭ supporter ✭, #313)
at least not unless every app can take them, but that's not what happens in android, only 'trusted' apps are allowed to take the wakelock.
in android they still use timeouts for most of userspace consuming events, and sidestep a huge amount of it by having the power controller have a wakelock for the entire time the screen backlight is on so that you don't have to scatter wakelocks through every app.
there are many of us who think that there are better ways of dealing with this userspace problem, but the problems of doing our own kernels makes it hard to experiment.
Posted Dec 23, 2011 23:25 UTC (Fri) by neilbrown (subscriber, #359)
Posted Dec 24, 2011 6:20 UTC (Sat) by dlang (✭ supporter ✭, #313)
even if I am trying to compile my own android kernel, there doesn't seem to be any way of installing it on a device in a way that will let me fall back to the older kernel if I run into problems with the new one (the equivalent to the old lilo -R option)
the integration of things into the mainline will greatly help the first problem, but I still don't know a good solution for the second one.
Posted Dec 24, 2011 11:50 UTC (Sat) by mjg59 (subscriber, #23239)
Posted Dec 24, 2011 12:30 UTC (Sat) by Cyberax (✭ supporter ✭, #52523)
It's _always_ possible even if your firmware is totally corrupted and even battery controller is dead. In some cases it requires a magic jig, though (a 101kOhm resistor shorting two USB lines).
Posted Mar 19, 2012 14:14 UTC (Mon) by jimmyflip (guest, #83547)
Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds