On Mon, 24 Oct 2011 12:23:43 +0200 "Rafael J. Wysocki" wrote: > On Monday, October 24, 2011, NeilBrown wrote: > > On Sun, 23 Oct 2011 15:16:36 +0200 "Rafael J. Wysocki" wrote: > > Similarly every system need one process to manage suspend. It can be my > > daemon or your daemon or Alan's daemon but it cannot be 2 or more of them > > running at the same time as that doesn't make any more sense than having > > systemd and init running at the same time. > > I agree that it doesn't makes sense. I don't agree that it implies people > won't try to do that. Does that matter? If they complain, tell them it isn't a supported configuration. > > > > > > > So I'd respond with "I'm not at all sure that we should throw away an > > > > > > all-userspace solution just yet". Particularly because many of us seem to > > > > > > still be working to understand what all the issues really are. > > > > > > > > > > OK, so perhaps we should try to implement two concurrent solutions, one > > > > > kernel-based and one purely in user space and decide which one is better > > > > > afterwards? > > > > > > > > Absolutely. > > > > > > > > My primary reason for entering this discussion is eloquently presented in > > > > http://xkcd.com/386/ > > > > > > > > Someone said "We need to change the kernel to get race-free suspend" and this > > > > simply is not true. I wanted to present a way to use the existing > > > > functionality to provide race-free suspend - and now even have code to do it. > > > > > > > > If someone else wants to write a different implementation, either in > > > > userspace or kernel that is fine. > > > > > > > > They can then present it as "I know this can be implemented in userspace, but > > > > I don't like that solution for reasons X, Y, Z and so here is my better > > > > kernel-space implementation" then that is cool. We can examine X, Y, Z and > > > > the code and see if the argument holds up. Maybe it will, maybe not. > > > > > > > > So far the only arguments I've seen for putting the code in the kernel are: > > > > > > > > 1/ it cannot be done in userspace - demonstrably wrong > > > > > > I'm not sure if that's correct. If you meant "it can be done in user space > > > without _any_ kernel modifications", I probably wouldn't agree. > > > > I have code to do it correctly today with no kernel modifications. It is > > called "lsusd". Proof by example. Or can you show that lsusd doesn't work > > correctly? > > So why do you consider making changes to the kernel (described in the other > part of the thread)? Are they completely cosmetic or are they needed for > functionality? Not needed. Maybe helpful. I have suggested three kernel changes - with varying levels of seriousness. 1/ Changes to wakeup_count so that it can be read without blocking. This is currently just a "general cleanliness" issue. It could become more of an issue if some kernel code activated a wakeup_source for a long time. It is not a problem at all for my current code, but if we wanted a single suspend daemon that didn't need threads or a helper process, then it might become an issue. 2/ Changes to flock locking so that a process can get notified when a lock attempt might succeed. This is just me grumbling about incomplete locking semantics and has nothing to do with power management directly. 3/ Activating a wakeup_source when an RTC alarm fires. This patch was proposed by John Stultz - I just supported it. It isn't strict necessary as the suspend daemon can check the RTC just before suspending and refuse to suspend in the alarm will fire in the next 2 seconds. However this assumes that the suspend will then complete within 2 seconds. This seems likely but I don't know that it is guaranteed. The 2 second window could be extended, but that isn't really ideal. So this is one kernel change that could be deemed to be "necessary". However it isn't really a chance in design at all - it just acknowledges that the RTC alarm is a wakeup source, so registers a wakeup_source for it, so it is really just a bug fix. I'm still interested to know what you think of this patch. While it isn't strictly needed I think it would be very helpful. (Without this, alarmtimers is racy too ... and it doesn't even insert a 2 second window .... I'm not really convinced alarmtimers is a good thing but it isn't clear that it is a bad thing either). NeilBrown