From: tytso on
On Mon, Jun 21, 2010 at 01:22:34PM +0100, Alan Cox wrote:
>
> You seem desperate to just throw it at Linus - you have been all along
> before the discussion, during it and now: but I don't understand why ?

Why are you so afraid to let Linus make the call? He's the benevolent
dictator, isn't he?

> It's as if you don't want progress to be made by other means ?

It doesn't look like progress is being made here.

- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
On Mon, 21 Jun 2010 09:42:49 -0400
tytso(a)mit.edu wrote:

> On Mon, Jun 21, 2010 at 01:22:34PM +0100, Alan Cox wrote:
> >
> > You seem desperate to just throw it at Linus - you have been all along
> > before the discussion, during it and now: but I don't understand why ?
>
> Why are you so afraid to let Linus make the call? He's the benevolent
> dictator, isn't he?

I don't mind him laughing at it. I'm just curious why you've tried to
short circuit the more general discussion repeatedly - even right at the
start.

> It doesn't look like progress is being made here.

On suspend blockers no - on some of the real problems yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Mon, 21 Jun 2010, Florian Mickler wrote:

> On Sun, 20 Jun 2010 22:23:38 -0400 (EDT)
> Alan Stern <stern(a)rowland.harvard.edu> wrote:

> > This is the race I was talking about:
> >
> > > > What happens if an event arrives just before you read
> > > > /sys/power/wakeup_count, but the userspace consumer doesn't realize
> > > > there is a new unprocessed event until after the power manager checks
> > > > it?
> >
> > > I think this is not the kernel's problem. In this approach the kernel makes it
> > > possible for the user space to avoid the race. Whether or not the user space
> > > will use this opportunity is a different matter.
> >
> > It is _not_ possible for userspace to avoid this race. Help from the
> > kernel is needed.
>
> It is possible if every (relevant) userspace program implements a
> callback for the powermanager to check if one of it's wakeup-sources
> got activated.
>
> That way the powermanager would read /sys/power/wakeup_count, then do
> the roundtrip to all it's registered users and only then suspend.
>
> This turns the suspend_blockers concept around. Instead of actively
> signaling the suspend_blockers, the userspace programs only answer
> "yes/no" when asked. (i.e. polling?)

In the end you would want to have communication in both directions:
suspend blockers _and_ callbacks. Polling is bad if done too often.
But I think the idea is a good one.

In fact, you don't need a "yes/no" response. Programs merely need a
chance to activate a new suspend blocker if a wakeup source was
recently activated before they acknowledge the poll.

> You _can not_ implement userspace suspend blockers with this approach,
> as it is vital for every userspace program to get scheduled and check
> it's wakeup-source (if even possible) before you know that the right
> parties have won the race.

I'm not sure what you mean. Certainly you can take a userspace
suspend-blocker implementation of the sort discussed before (where
programs communicate their needs to a central power-manager process)
and add this callback mechanism on top.

There is still at least one loophole to be closed: Android's
timer-based wakelocks. These include cases where the Android
developers didn't add enough wakelocks to cover the entire path from
kernel-event to userspace-handler, so they punted and relied on a timer
to decide when the wakelock should be deactivated. (There may be other
cases too; I didn't follow the original discussion very closely.)
It's not clear whether these things can be handled already in Rafael's
scheme with your addition, or whether something new is needed.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Sun, 20 Jun 2010, mark gross wrote:

> Your confused about what problem this patch attempts to solve.

I don't think so. Rafael's description was pretty clear.

> There is
> a pm_qos patch in the works to address the suspend blocker
> functionality.
> http://lists.linux-foundation.org/pipermail/linux-pm/2010-June/026760.html

No. That patch addresses something _similar_ to the suspend blocker
functionality. The fact remains, though, that pm_qos is not used
during system suspend (the /sys/power/state interface), hence changes
to pm_qos won't solve the system-suspend problems that suspend blockers
do solve.

Alan Stern

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