Re: [RFC][PATCH 0/2] PM / Sleep: Extended control of suspend/hibernate interfaces

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 4613 bytes --]

On Mon, 24 Oct 2011 12:23:43 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Monday, October 24, 2011, NeilBrown wrote:
> > On Sun, 23 Oct 2011 15:16:36 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]