Re: [RFC][PATCH 0/2] PM / Sleep: Extended control ofsuspend/hibernate interfaces
Date: Mon Oct 24 2011 - 22:52:53 EST
On Mon, 24 Oct 2011 12:23:43 +0200 "Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
> On Monday, October 24, 2011, NeilBrown wrote:
> > On Sun, 23 Oct 2011 15:16:36 +0200 "Rafael J. Wysocki" <rjw@xxxxxxx> 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
> > > > > > 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
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
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).
Description: PGP signature