Date:	Mon, 21 Jun 2010 11:57:21 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	markgross@...gnar.org
cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	<linux-pm@...ts.linux-foundation.org>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Arve Hjønnevåg <arve@...roid.com>,
	Neil Brown <neilb@...e.de>, mark gross <640e9920@...il.com>
Subject: Re: [RFC][PATCH] PM: Avoid losing wakeup events during suspend

On Sun, 20 Jun 2010, mark gross wrote:

> > > 1) I don't think suspend blockers really solve or effectively articulate
> > > the suspend wake event race problem.
> > 
> > Why not?
> 
> For starters the suspend block patch is about suspend blocking, at least
> at first before competing ideas starting coming out and this race issue
> was offered up as an excuse to not consider the alternative ideas, then
> suddenly suspend blocking became a wake event notification mechanism
> implementation that was baked into the implementation.  The arguments
> are still a blur and irritating to recall / look up again.

I get the feeling you didn't fully absorb the intent of the original
wakelock patches.  Right from the start they were about fixing a race
in system suspend: The system may go to sleep even though a pending
wakeup event should have blocked or prevented the suspend.  In
particular that means notifying the PM core about wakeup events, when
they occur, and when the system may once again be allowed to suspend.

The userspace parts of the original patches did cloud the issue,
admittedly.  But the purpose of the in-kernel parts has always been
clear.

> But, the point I'm trying to make is that wake event serialization /
> event handling suddenly became a big FUD-pie tightly bound to a specific
> feature for suspend blocking (or was is auto suspending?  Its a magical
> solution to all the PM problems in the kernel.  I'm being sarcastic.)  
> 
> Its much better to call out the issue and provide a clean targeted
> solution to it without binding it to some other solution to a different
> problem.

That's exactly what the wakelock patches did:  They called out the
issue of the system suspending even while there were pending wakeup
events, they provided a targeted solution, and it wasn't bound to
another solution to a different problem.

Indeed, if you go back through the (I agree, irritating) threads on
this topic, you'll see that the FUD and other issues were all
introduced by other kernel developers, mainly because they didn't like
the idea of using system suspend as a mechanism for dynamic power
management.

> > > I don't think suspend-blocker handles both kinds of races as well as you
> > > seem to think.
> > 
> > Can you give any counterexamples?
> 
> I knew you'd ask such a thing.  Do you have any correctness proofs of it
> working right?

If you want, sure.  But what you think "working right" means may not be 
the same as what I think.

> Lets consider the RIL incoming call race:

"RIL"?

> 1) user mode initiates an "opportunistic suspend" or whatever we call
> this these days by writing to some sys interface.

Okay.

> 2) a call comes in (multi-threaded cpu) just after the write.

Right.  Presumably we want the suspend to be delayed until the call 
can be handled by a user programm, yes?

> 3) the caif driver grabs a blocker, stopping the in flight suspend in the
> nick of time.  But, when should it release it without racing?  How does
> one implement that kernel code such that its portable to non-android user
> mode stacks? 

I don't know anything about the caif driver so I can't answer this
question in detail.  Here's the general idea: Somehow the kernel has to
notify userspace that an incoming call has arrived.  Whatever driver or
subsystem sends that notification should release the suspend blocker
once userspace indicates that the notification has been received (for
example, by reading all the data in an input event queue).  That's true
for both Android and non-Android.

In some cases there may not be any mechanism for userspace to tell the 
kernel when a notification has been received.  For thoses cases, the 
Android people used timer-based blockers.  This is not necessarily the 
best approach but they seem happy with it.  Others might prefer to add 
an explicit "notification received" mechanism.

Still others take the view that since suspends are initiated by 
userspace anyway, it's not necessary to tell the kernel when suspend is 
safe again.  Just tell the user process responsible for initiating 
suspends.

> > >  A single tool that conflates multiple kernel facilities
> > > is not an advantage.  It limits applications outside of the scope of
> > > doing it the "android way".
> > 
> > I don't see that necessarily as a drawback.  You might just as well say
> > that the entire Linux kernel limits applications to doing things the
> > "Unix" way.  Often have fewer choices is an advantage.
> 
> I disagree with your analogy.  One problem one solution with minimal
> coupling to other implementation details is nice.  Two problems with one
> solution dependent on the system wide architecture bound to a user mode
> PM design is fragile and not portable.

I assume the "two problems" you refer to are: telling the PM core that 
the kernel has a wakeup event in progress, and telling the PM core that 
userspace has a wakeup event in progress.  To me these don't seem to be 
vastly different problems, and having a single solution for both 
doesn't seem out of line.

The fact that this is bound to a user-mode PM design was inevitable, 
given the whole idea was to overcome weaknesses in the system suspend 
implementation and that implementation already is driven by userspace.

> > (Incidentally, even on Android the centralized PM process does not 
> > handle _all_ of the userspace/kernel wakelock interactions.)
> >
> 
> Yeah, I've been told that too.  I've grepped for where the wake_unlock
> sysfs interfaces are hit from user mode android (eclair) and I would
> like some help in identifying those guys. Maybe its in the FroYo code I
> don't get to look at yet?
> 
> There is libhardware_legacy/power/power.c and  an out of tree kernel
> broadcom bcm4329 driver under hardware/broadcom but that doesn't count
> as it looks like a kernel driver to me.

I don't know -- I have never looked at the Android userspace.  No doubt 
Arve can provide some examples.

Alan Stern

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