From: Matthew Garrett on
On Thu, May 27, 2010 at 10:23:50PM +0200, Thomas Gleixner wrote:
> On Thu, 27 May 2010, Matthew Garrett wrote:
> > A wakeup event is defined as one that wakes the system - if a system
> > can't be woken by a specific event then it's impossible to lose it,
> > since it wasn't a wakeup event to begin with.
>
> So where is the problem ?

The problem is that, right now, if a wakeup event is received between
the point where userspace decides to start a suspend and userspace
actually starts a suspend, that event may not abort the suspend.

> > Consider the case where the read() is non-blocking.
>
> Which I consider in the same range as the application which does:
>
> while (1);

Not at all. Depending on what it reads, it may follow some other path
where it sleeps. But, as I keep saying, if you don't want to support
that kind of code then all of this is massively easier.

> > What sets that flag, and how does it interact with an application that
> > never makes a blocking system call?
>
> QoS(NONE) would be default policy for untrusted apps in the same way
> as you'd refuse the usage of supsend blockers for untrusted apps.
>
> while (1); is definitely not an application which should be granted
> trusted rights, right ?

No, but if that while (1) is draw_cows() then the user may want this to
run while their session is active and stop running while their session
is idle. So you only want it to be QoS(NONE) in the idle session case.
How do you change that state?

> > The numbers were earlier in the thread.
>
> Numbers, yes. But I really give a sh*t about numbers w/o a detailed
> explanation of the scenario which created those numbers. And if the
> numbers boil down to: we handle the untrusted app which does "while
> (1);" then they are absolutely useless.

The tested case was a stock Android install with opportunistic suspend
enabled and one that just used runtime idle. The lowest power state
entered was the same on both.

> > Yes, and I'd agree with this if anyone seemed to have any idea how to do
> > it right. But despite all the heat and noise in this thread, all we've
> > got is an expression that it should be handled by the scheduler (a
> > viewpoint I agree with) without any explanation of how to switch
> > policies in such a way that applications idle without losing wakeup
> > events.
>
> Why in the world should they lose wakeup events ? If an app goes idle,
> it goes idle because it is waiting for an event. There is no other
> sane reason except for those apps which are untrusted and force
> idled. And for those you agreed already the suspend blockers don't
> solve anything as they are either not implemented or the app is not
> trusted to use them.
>
> So we are back to round one of the whole discussion:
>
> Is it worth to create an utter mess in the kernel just to handle a
> subset of crappy coded applications ?
>
> And the answer stays the same: No, not at all.

You need suspend blockers to avoid losing wakeups in the explicit
suspend case even if you don't want to implement opportunistic suspend.

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
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
> > The event was not lost.
>
> Not lost, but not delivered. So you need your policy agent to send

it will be delivered next time the process wakes. It's not lost.

> SIGCONT when you receive any wakeup event, which either means proxying
> all your network traffic through your policy agent or having some
> mechanism for alerting the policy agent whenever you leave the deep idle
> state.

You didn't mention that requirement last time.

> > > also the race of an application being in the middle of handling a wakeup
> > > event when you send it the signal.
> >
> > sigmask()
>
> Doesn't help - I may be hit by the signal between the poll() unblocking
> and me having the opportunity to call sigmask().

ppoll(). This is all existing solved stuff.

Alan
--
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 Thu, 27 May 2010 18:59:20 +0100
Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:

> On Thu, May 27, 2010 at 07:56:21PM +0200, Peter Zijlstra wrote:
> > On Thu, 2010-05-27 at 18:52 +0100, Matthew Garrett wrote:
> >
> > > If that's what you're aiming for then you don't need to block
> > > applications on hardware access because they should all already have
> > > idled themselves.
> >
> > Correct, a well behaved app would have. I thought we all agreed that
> > well behaved apps weren't the problem?
>
> Ok. So the existing badly-behaved application ignores your request and
> then gets blocked. And now it no longer responds to wakeup events. So
> you penalise well-behaved applications without providing any benefits to
> badly-behaved ones.

I don't see how you put the first two sentences together and get the
final one.

When you beat up badly behaved apps that doesn't penalise well behaved
ones.

Forcing "well behaved apps" to make hundreds of extra calls to a complex
blocker interface that also requires tons of kernel code and requires the
application know platform policy and be recompiled if it changes - now
that is punishing well behaved apps.

A well behaved app should just work using standard existing APIs because
that is how all the standard current well behaved apps are written [1].

Alan
--
[1] I'm dying to see the suspend blocker patch for evolution ;)
--
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: Rafael J. Wysocki on
On Thursday 27 May 2010, Thomas Gleixner wrote:
> On Thu, 27 May 2010, Alan Stern wrote:
> > On Thu, 27 May 2010, Peter Zijlstra wrote:
> >
> > > On Thu, 2010-05-27 at 16:33 +0100, Alan Cox wrote:
> > > > On Thu, 27 May 2010 17:09:16 +0200
> > > > Peter Zijlstra <peterz(a)infradead.org> wrote:
> > > >
> > > > > On Thu, 2010-05-27 at 11:06 -0400, Alan Stern wrote:
> > > > > >
> > > > > > Opportunistic suspends are okay.
> > > > > >
> > > > > > The proposed userspace API is too Android-specific.
> > > > >
> > > > > I would argue opportunistic suspends are not ok, and therefore the
> > > > > proposed API is utterly irrelevant.
> > > >
> > > > Assuming you are happy that opportunistically entering C6 and the like is
> > > > ok via ACPI can you explain why you have a problem with opportunistic
> > > > suspend and why it is different to a very deep sleep CPU state such as we
> > > > have now (and which on many embedded platforms we deal with *is* sleeping
> > > > external devices too)
> > >
> > > Agreed, but I understood the opportunistic suspend line from Alan Stern
> > > to mean the echo opportunistic > /sys/power/foo thing.
> >
> > Yes, that's what I meant. Why do you think it is any worse than "echo
> > mem >/sys/power/state"? The only difference is that it will block
> > until all in-kernel suspend blockers are disabled.
> >
> > Or do you also think that "echo mem >/sys/power/state" is evil and
> > should be removed from the kernel as soon as possible?
> >
> > And to answer Thomas's question: The whole reason for having in-kernel
> > suspend blockers is so that userspace can tell the system to suspend
> > without losing wakeup events.
> >
> > Suppose a key is pressed just as a user program writes "mem" to
> > /sys/power/state. The keyboard driver handles the keystroke and queues
> > an input event. Then the system suspends and doesn't wake up until
> > some other event occurs -- even though the keypress was an important
> > one and it should have prevented the system from suspending.
> >
> > With in-kernel suspend blockers and opportunistic suspend, this
> > scenario is prevented. That is their raison-d'etre.
>
> I tend to disagree. You are still looking at suspend as some extra
> esoteric mechanism. We should stop doing this and look at suspend (to
> RAM) as an additional deep idle state, which is well defined in terms
> of power savings and wakeup latency.

Well, the ACPI spec doesn't agree with you. :-)

> That's what I think opportunistic suspend is all about. Now if you look at
> our current idle states in x86 the incoming keystroke is never lost.
>
> So when suspend does lose the wakeup event then we need to fix that,

With ACPI, we can't, because we have to switch our wakeup sources from
the "runtime" mode to the "system wakeup" mode in a racy fashion.

> but why do we need suspend blockers for this ? Let's take your example:
>
> > The keyboard driver handles the keystroke and queues an input
> > event. Then the system goes into suspend ....
>
> Why do we go into suspend at all?

The decision to go to suspend may be made in parallel to the keystroke in
such a way that we may not be able to detect it (that doesn't apply to the
hardware Android currently runs on, though).

> If there is an event queued then something is woken up and got running,
> which is reason enough _not_ to enter suspend. If that's broken in the
> current implementation then we need to fix it,

That's not a matter of implementation (which I admit can be improved).

> but not with a big hammer by adding another
> interface. We have that information already and obviously we do not
> use it, so lets figure out why before adding suspend blockers just
> because they paper over the underlying problem.

I'm not really sure what information you're referring to.

Rafael
--
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: Matthew Garrett on
On Thu, May 27, 2010 at 10:03:14PM +0100, Alan Cox wrote:
> On Thu, 27 May 2010 18:59:20 +0100
> Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> > Ok. So the existing badly-behaved application ignores your request and
> > then gets blocked. And now it no longer responds to wakeup events. So
> > you penalise well-behaved applications without providing any benefits to
> > badly-behaved ones.
>
> I don't see how you put the first two sentences together and get the
> final one.
>
> When you beat up badly behaved apps that doesn't penalise well behaved
> ones.

If you're going to block an app on drawing then you either need to
reenable drawing on wakeup or you need to have an interface for alerting
the app to the fact that drawing is about to block and it should get
back to its event loop. The first is suboptimal, the second penalises
well behaved apps.

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
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/