Message-Id: <201110082057.43020.rjw@sisk.pl>
Date:	Sat, 8 Oct 2011 20:57:42 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	markgross@...gnar.org
Cc:	NeilBrown <neilb@...e.de>, linux-kernel@...r.kernel.org,
	John Stultz <john.stultz@...aro.org>, 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 Saturday, October 08, 2011, mark gross 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.
> 
> > If user-space wants to prevent suspend, it just needs some sort of protocol
> > for talking to the user-space process which follows the correct protocol to
> > initiate suspend.  That isn't a kernel problem.
> 
> The devices that I've seen that need to block suspend don't have a
> communication interface to user mode. 
> 
> But, you are right the devices that need this sort of thing could
> register as wakeup sources and block suspend as well.
> 
> FWIW This part of the patch set was to wrap up a proposal I got last
> year from some folks to try to provide somewhat compatible semantics to
> wakelock's for the android and linux kernel community.
> 
> I include it for completeness. 
> > 
> > > 
> > > 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.

This, in fact, isn't correct.  I have tried to turn your (and John's)
attention to this for quite a few times already.

The point is that the entity about to trigger suspend (that need not be the
kernel!) has to communicate with the processes that consume wakeup events
beforehand.  In theory this communication can happen entirely in user
space, but that would involve quite complicated interactions between
processes, so nobody does that in practice.

The only "problem" that can't be solved entirely in user space, which is
what John turned my attention to during the LPC, is that it may be
possible to suspend when processes that should be asked about whether or
not to suspend are sleeping and that may be done _without_ actually asking
those processes for permission.  The difficult part is, if we suspend in
such a situation, we need to wait until all of those processes have a chance
to run before attempting to suspend again.

> The fact that there are time outs in the existing wake event code points to
> this shortcoming in the current implementation.

Actually, the timeouts serve a different purpose.  Namely, there are wakeup
events that aren't actually consumed by anyone above the layer signaling the
event (think about Wake-on-LAN via a magic packet) and if such an event
happens, we can't suspend at once, because we need to assume that it happened
for a reason, so whoever triggered the event has to be given a chance to do
whatever he needed to wake up the system for.  This cannot be achieved without
timeouts.
 
> 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.

Agreed.

> 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?
> 
> > 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.

That's correct.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists