PM: Avoid losing wakeup events during suspend
in [Kernel]
|
Prev: [PATCH] [RFC] kvm: time accelerator/decelerator for kvm guests
Next: wait_for_completion_interruptible does not wait !!
From: mark gross on 21 Jun 2010 21:10 On Mon, Jun 21, 2010 at 08:10:58AM -0400, tytso(a)mit.edu wrote: > So where are we at this point? The patches are in James Bottomly's tree. > Discussion had completely died down for a while, and it's picked up > again, but it's not clear to me that we're any closer to reaching > consensus. I thought we (linux community and Android) where ok with the plist / pm-qos implementation of the building blocks needed to implement the suspend blocker feature on top of a pm-qos request class (I think the name was "interactive") pretty much the exact same symantecs as the suspend blocker thing, just with pm-qos kernel api's. > There's been one proposal that we simply merge in a set of no-op > inline functions for suspend blockers, just so we can get let the > drivers go in (assuming that Greg K-H believes this is still a > problem), but with an automatical removal of N months (N to be > decided, say 9 or 12 or 18 months). I'd rather see the re-tooling of pmqos happen. > > My concern is that if we do that, we will have simply kicked the ball > down the road for N months. Another approach is to simply merge in > no-op functions and not leave any kind of deprecation schedule. > That's sort of an implicit admission of the fact that we may not reach > consensus on this issue. Or we could simply ship the patches as-is to > Linus after he gets back from vacation and ask him for a thumbs up or > thumbs down vote, which might settle things once and for all. > > How do we go forward from here? > put the pm_qos -plist update into linux-next? --mgross -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: mark gross on 21 Jun 2010 21:30 On Mon, Jun 21, 2010 at 12:01:09PM -0400, Alan Stern wrote: > On Sun, 20 Jun 2010, mark gross wrote: > > > Your confused about what problem this patch attempts to solve. > > I don't think so. Rafael's description was pretty clear. Then how is it you don't understand the fact that Rafael's patch is to solve the wake event notification suspend race and not block opertunistic suspends or kernel critical sections where suspending should be disabled? > > > There is > > a pm_qos patch in the works to address the suspend blocker > > functionality. > > http://lists.linux-foundation.org/pipermail/linux-pm/2010-June/026760.html > > No. That patch addresses something _similar_ to the suspend blocker > functionality. The fact remains, though, that pm_qos is not used > during system suspend (the /sys/power/state interface), hence changes > to pm_qos won't solve the system-suspend problems that suspend blockers > do solve. You keep saying they solve something, I keep wondering what you are talking aobut. Lets see what problems it solves: * implements oppertunistic suspending (this is a feature not a problem) * enables kernel critical sections blocking suspending. * requiers overlapping application specific critcal sections from ISR into user mode to make implementation correct. * exposes a user mode interface to set a critical section. * reduces races between wake events (or suspend blocking events) but I'm not convinced it solves them. suspend blockers provide a way to block oppertunistic suspending, wich I'll have you know, is a pain to get working right and the enabling from device to device is not very portable and *that* doesn't say good things about the scheme. --mgross -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: mark gross on 21 Jun 2010 22:00 On Mon, Jun 21, 2010 at 11:57:21AM -0400, Alan Stern wrote: > On Sun, 20 Jun 2010, mark gross wrote: > > > > > 1) I don't think suspend blockers really solve or effectively articulate > > > > the suspend wake event race problem. > > > > > > Why not? > > > > For starters the suspend block patch is about suspend blocking, at least > > at first before competing ideas starting coming out and this race issue > > was offered up as an excuse to not consider the alternative ideas, then > > suddenly suspend blocking became a wake event notification mechanism > > implementation that was baked into the implementation. The arguments > > are still a blur and irritating to recall / look up again. > > I get the feeling you didn't fully absorb the intent of the original > wakelock patches. Right from the start they were about fixing a race > in system suspend: The system may go to sleep even though a pending wrong. They are about 1) adding opportunistic suspend, and 2) adding critical sections to block suspend. No race issues where ever talked about until alternative solutions where put up. > wakeup event should have blocked or prevented the suspend. In > particular that means notifying the PM core about wakeup events, when > they occur, and when the system may once again be allowed to suspend. > > The userspace parts of the original patches did cloud the issue, > admittedly. But the purpose of the in-kernel parts has always been > clear. > > > But, the point I'm trying to make is that wake event serialization / > > event handling suddenly became a big FUD-pie tightly bound to a specific > > feature for suspend blocking (or was is auto suspending? Its a magical > > solution to all the PM problems in the kernel. I'm being sarcastic.) > > > > Its much better to call out the issue and provide a clean targeted > > solution to it without binding it to some other solution to a different > > problem. > > That's exactly what the wakelock patches did: They called out the > issue of the system suspending even while there were pending wakeup > events, they provided a targeted solution, and it wasn't bound to > another solution to a different problem. > > Indeed, if you go back through the (I agree, irritating) threads on > this topic, you'll see that the FUD and other issues were all > introduced by other kernel developers, mainly because they didn't like > the idea of using system suspend as a mechanism for dynamic power > management. I am struck by how differently you are seeing things. > > > > > I don't think suspend-blocker handles both kinds of races as well as you > > > > seem to think. > > > > > > Can you give any counterexamples? > > > > I knew you'd ask such a thing. Do you have any correctness proofs of it > > working right? > > If you want, sure. But what you think "working right" means may not be > the same as what I think. > > > Lets consider the RIL incoming call race: > > "RIL"? You are not an android developer are you? RIL is the user mode Radio Interface Layer. > > 1) user mode initiates an "opportunistic suspend" or whatever we call > > this these days by writing to some sys interface. > > Okay. > > > 2) a call comes in (multi-threaded cpu) just after the write. > > Right. Presumably we want the suspend to be delayed until the call > can be handled by a user programm, yes? > > > 3) the caif driver grabs a blocker, stopping the in flight suspend in the > > nick of time. But, when should it release it without racing? How does > > one implement that kernel code such that its portable to non-android user > > mode stacks? > > I don't know anything about the caif driver so I can't answer this > question in detail. Here's the general idea: Somehow the kernel has to > notify userspace that an incoming call has arrived. Whatever driver or > subsystem sends that notification should release the suspend blocker > once userspace indicates that the notification has been received (for > example, by reading all the data in an input event queue). That's true > for both Android and non-Android. so tell me how user space indicates to the kernel object will ever know when its ok to release the blocker? then tell me how suspend blocker provide an elegant portable solution to this that works for multiple user mode stacks. > > In some cases there may not be any mechanism for userspace to tell the > kernel when a notification has been received. For thoses cases, the > Android people used timer-based blockers. This is not necessarily the > best approach but they seem happy with it. Others might prefer to add > an explicit "notification received" mechanism. the time-based blockers are not part of the latest patch set. > > Still others take the view that since suspends are initiated by > userspace anyway, it's not necessary to tell the kernel when suspend is > safe again. Just tell the user process responsible for initiating > suspends. > > > > > A single tool that conflates multiple kernel facilities > > > > is not an advantage. It limits applications outside of the scope of > > > > doing it the "android way". > > > > > > I don't see that necessarily as a drawback. You might just as well say > > > that the entire Linux kernel limits applications to doing things the > > > "Unix" way. Often have fewer choices is an advantage. > > > > I disagree with your analogy. One problem one solution with minimal > > coupling to other implementation details is nice. Two problems with one > > solution dependent on the system wide architecture bound to a user mode > > PM design is fragile and not portable. > > I assume the "two problems" you refer to are: telling the PM core that > the kernel has a wakeup event in progress, and telling the PM core that > userspace has a wakeup event in progress. To me these don't seem to be > vastly different problems, and having a single solution for both > doesn't seem out of line. nope. problem 1) opportunistic enable suspend deferral for critical sections when suspending is "bad" problem 2) race between entering pm_suspend call tree and wake events. > The fact that this is bound to a user-mode PM design was inevitable, > given the whole idea was to overcome weaknesses in the system suspend > implementation and that implementation already is driven by userspace. I think we can do better. > > > (Incidentally, even on Android the centralized PM process does not > > > handle _all_ of the userspace/kernel wakelock interactions.) > > > > > > > Yeah, I've been told that too. I've grepped for where the wake_unlock > > sysfs interfaces are hit from user mode android (eclair) and I would > > like some help in identifying those guys. Maybe its in the FroYo code I > > don't get to look at yet? > > > > There is libhardware_legacy/power/power.c and an out of tree kernel > > broadcom bcm4329 driver under hardware/broadcom but that doesn't count > > as it looks like a kernel driver to me. > > I don't know -- I have never looked at the Android userspace. No doubt > Arve can provide some examples. who are you? You don't know about the Android stack, and you keep blowing about how suspend blockers solve all the PM problems? I'm starting to think your just trolling. --mgross -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 21 Jun 2010 22:30 On Mon, 21 Jun 2010, mark gross wrote: > On Mon, Jun 21, 2010 at 12:01:09PM -0400, Alan Stern wrote: > > On Sun, 20 Jun 2010, mark gross wrote: > > > > > Your confused about what problem this patch attempts to solve. > > > > I don't think so. Rafael's description was pretty clear. > > Then how is it you don't understand the fact that Rafael's patch is to > solve the wake event notification suspend race and not block opertunistic > suspends or kernel critical sections where suspending should be disabled? I don't know what gave you the idea that I think Rafael's patch is meant to block kernel critical sections. I certainly don't think that. However leaving that aside, the rest of the above is just two different ways of saying the same thing: Wakeup events should cause suspend to be disabled until the events are processed. Arrival of wakeup events races with initiation of system suspend (whether opportunistic or not). Therefore blocking suspends when they ought to be disabled requires us to solve the wake event/suspend race. You can't do one without doing the other. > > > There is > > > a pm_qos patch in the works to address the suspend blocker > > > functionality. > > > http://lists.linux-foundation.org/pipermail/linux-pm/2010-June/026760.html > > > > No. That patch addresses something _similar_ to the suspend blocker > > functionality. The fact remains, though, that pm_qos is not used > > during system suspend (the /sys/power/state interface), hence changes > > to pm_qos won't solve the system-suspend problems that suspend blockers > > do solve. > > You keep saying they solve something, I keep wondering what you are > talking aobut. > Lets see what problems it solves: > * implements oppertunistic suspending (this is a feature not a problem) > * enables kernel critical sections blocking suspending. > * requiers overlapping application specific critcal sections from ISR > into user mode to make implementation correct. > * exposes a user mode interface to set a critical section. > * reduces races between wake events (or suspend blocking events) but I'm > not convinced it solves them. The last item on your list is what I meant. The others are not problems solved by suspend blockers. Well, maybe the second is, but I don't know of any kernel critical sections that need to block suspend other than those caused by wakeup events. I agree with you that bundling opportunistic suspend and the user mode interface together with suspend blockers made the situation more difficult by mixing up the important issues. > suspend blockers provide a way to block oppertunistic suspending, wich > I'll have you know, is a pain to get working right and the enabling from > device to device is not very portable and *that* doesn't say good things > about the scheme. The real problem with the scheme is to define which events should count as "wakeup" events and to determine when they have been fully handled. I can see that these might well vary from one platform to another. (Maybe that's what you mean by saying the enabling is not very portable.) But these issues are unavoidable; any scheme will have to address them. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 21 Jun 2010 22:50
On Mon, 21 Jun 2010, mark gross wrote: > > I get the feeling you didn't fully absorb the intent of the original > > wakelock patches. Right from the start they were about fixing a race > > in system suspend: The system may go to sleep even though a pending > > wrong. They are about 1) adding opportunistic suspend, and 2) adding > critical sections to block suspend. > > No race issues where ever talked about until alternative solutions > where put up. That's not how I remember it. But this point isn't important enough to be worth digging through the old emails to verify or disprove it. > I am struck by how differently you are seeing things. Those discussions were (and still are!) so long and annoying, it wouldn't be surprising if _everybody_ saw them differently! :-) You certainly must agree that a lot of differing personal viewpoints were expressed. > > > Lets consider the RIL incoming call race: > > > > "RIL"? > > You are not an android developer are you? Nope. > RIL is the user mode Radio > Interface Layer. > > > > 1) user mode initiates an "opportunistic suspend" or whatever we call > > > this these days by writing to some sys interface. > > > > Okay. > > > > > 2) a call comes in (multi-threaded cpu) just after the write. > > > > Right. Presumably we want the suspend to be delayed until the call > > can be handled by a user programm, yes? > > > > > 3) the caif driver grabs a blocker, stopping the in flight suspend in the > > > nick of time. But, when should it release it without racing? How does > > > one implement that kernel code such that its portable to non-android user > > > mode stacks? > > > > I don't know anything about the caif driver so I can't answer this > > question in detail. Here's the general idea: Somehow the kernel has to > > notify userspace that an incoming call has arrived. Whatever driver or > > subsystem sends that notification should release the suspend blocker > > once userspace indicates that the notification has been received (for > > example, by reading all the data in an input event queue). That's true > > for both Android and non-Android. > > so tell me how user space indicates to the kernel object will ever know > when its ok to release the blocker? First you tell me how the kernel indicates to userspace that an incoming call has arrived. My answer will depend on that. > then tell me how suspend blocker provide an elegant portable solution > to this that works for multiple user mode stacks. When did I -- or anyone -- ever say it was "elegant"? As for multiple user mode stacks, I don't see the issue. If you grant there is a mechanism whereby userspace indicates to the kernel that a blocker may be released, then it seems obvious that this mechanism could be used in multiple user mode stacks. > problem 1) opportunistic enable suspend deferral for critical sections > when suspending is "bad" > problem 2) race between entering pm_suspend call tree and wake events. Can you point out any examples of kernel critical sections where suspending is "bad" that aren't started by arrival of a wakeup event? Unless you can, I'm justified in saying that these "two" problems are one and the same. > > The fact that this is bound to a user-mode PM design was inevitable, > > given the whole idea was to overcome weaknesses in the system suspend > > implementation and that implementation already is driven by userspace. > > I think we can do better. Maybe we can. I'm certainly not going to claim that suspend blockers are the best possible solution. And I'm not really keen on seeing them merged along with all the attendant opportunistic suspend and userspace API stuff. But so far I haven't heard anything significantly better. > > I don't know -- I have never looked at the Android userspace. No doubt > > Arve can provide some examples. > who are you? You sound like a Vorlon. Should I ask: "What do you want?" :-) > You don't know about the Android stack, and you keep > blowing about how suspend blockers solve all the PM problems? Not all of them. Just the race between wakeup events and suspend. > I'm > starting to think your just trolling. You may think what you like. Perhaps there is a certain degree of truth to the accusation -- goodness knows, I do tend to prolong technical conversations when I think I'm right. (Ask anybody who has corresponded with me for any length of time.) But this is probably true of a lot of kernel developers... Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |