Message-ID: <20111009093100.6c15be50@notabene.brown>
Date:	Sun, 9 Oct 2011 09:31:00 +1100
From:	NeilBrown <neilb@...e.de>
To:	markgross@...gnar.org
Cc:	linux-kernel@...r.kernel.org, John Stultz <john.stultz@...aro.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>, arve@...roid.com,
	Alan Stern <stern@...land.harvard.edu>,
	amit.kucheria@...aro.org, farrowg@...ibm.com,
	"Dmitry Fink (Palm GBU)" <Dmitry.Fink@...m.com>,
	linux-pm@...ts.linux-foundation.org, khilman@...com,
	Magnus Damm <damm@...nsource.se>, mjg@...hat.com,
	peterz@...radead.org
Subject: Re: [markgross@...ngar.org: [RFC] wake up notifications and suspend
 blocking (aka more wakelock stuff)]

On Sat, 8 Oct 2011 11:16:38 -0700 mark gross <markgross@...gnar.org> wrote:

> On Sat, Oct 08, 2011 at 10:14:39PM +1100, NeilBrown wrote:
> > On Sun, 2 Oct 2011 09:44:56 -0700 mark gross <markgross@...gnar.org> wrote:
> > 
> > > resending to wider list for discussion
> > > ----- Forwarded message from mark gross <markgross@...ngar.org> -----
> > > 
> > > Subject: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)
> > > Date: Tue, 20 Sep 2011 13:33:05 -0700
> > > From: mark gross <markgross@...ngar.org>
> > > To: linux-pm@...ts.linux-foundation.org
> > > Reply-To: markgross@...gnar.org
> > > Cc: arve@...roid.com, markgross@...gnar.org, Alan Stern <stern@...land.harvard.edu>, amit.kucheria@...aro.org, farrowg@...ibm.com, "Rafael J. Wysocki" <rjw@...k.pl>
> > > 
> > > The following patch set implement an (untested) solution to the
> > > following problems.
> > > 
> > > 1) a method for making a system unable to suspend for critical sections
> > > of time.
> > 
> > We already have this.  A properly requested suspend (following wakeup_count
> > protocol) is unable to complete between wakeup_source_activate() and
> > wake_source_deactivate() - these delimit the critical sections.
> > 
> > What more than this do you need?
> 
> sometimes devices that are not wake up sources need critical sections
> where suspend is a problem.

I agree with Alan that an example would help here.
My naive perspective is that any device is either acting on behalf of a
user-space program, so it should disable suspend, or on behalf of some
external event, so that event is ipso-facto a wakeup event.

> > 
> > > 
> > > 2) providing a race free method for the acknowledgment of wake event
> > > processing before re-entry into suspend can happen.
> > 
> > Again, this is a user-space problem.  It is user-space which requests
> > suspend.  It shouldn't request it until it has checked that there are no wake
> > events that need processing - and should use the wakeup_count protocol to
> > avoid races with wakeup events happening after it has checked.
> 
> Here you are wrong, or missing the point.  The kernel needs to be
> notified from user mode that an update event has been consumed by
> whoever cares about it before the next suspend can happen.  The fact
> that there are time outs in the existing wake event code points to this
> shortcoming in the current implementation.

 ... or I have a different perspective.
A write to wakeup_count is a notification to the kernel that all wakeup
events that had commenced prior to that same number being read from
wakeup_count have been consumed.

So we already have a mechanism for the notification that you want.

> 
> I suppose one could rig up the user mode suspend daemon with
> notification callbacks between event consumers across the user mode
> stack but its really complex to get it right and forces a solution to a
> problem better solved in kernel mode be done with hacky user mode
> gyrations that may ripple wildly across user mode.

I suspect it is in here that the key to our different perspectives lies.
I think than any solution must "ripple wildly across user mode" if by that
you mean that more applications and daemons will need to be power-aware and
make definitive decisions about when they cannot tolerate suspend.
Whether those apps and daemons tell the kernel "don't suspend now" or tell
some user-space daemon "don't suspend now" is fairly irrelevant when
assessing the total impact on user-space.

I think a fairly simple protocol involving file locking can be perfectly
adequate to communicate needs relating to suspend-or-don't-suspend among
user-space processes.


> 
> Also it is the kernel that is currently deciding when to unblock the
> suspend daemon for the next suspend attempt.  Why not build on that and
> make is so we don't need the time outs?

Suspend is a joint decision by user-space and kernel-space.  Each part should
participate according to its expertise.
The kernel can make use of information generated by drivers in the kernel.
User-space can consolidate information generated by user-space processes.


> 
> > i.e. there is no kernel-space problem to solve here (except for possible
> > bugs).
> 
> Just a race between the kernel allowing a suspend and the user mode code
> having time to consume the last wake event.
>

Providing that the source of the wake event does not deactivate the
wakeup_source before the event is visible to userspace, this race is easily
avoided in userspace:

   - read wakeup_count
   - check all possible wakeup events.
   - if there were none, write back to wakeup_count and request a suspend.

This is race-free.

If some wakeup_source is deactivated before the event is visible to
user-space, then that is a bug and should be fixed.
If there is some particular case where it is non-trivial to fix that bug,
then that would certainly be worth exploring in detail.

NeilBrown


Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)

Powered by blists - more mailing lists