Message-ID: <1275349638.2823.395.camel@mulgrave.site>
Date:	Mon, 31 May 2010 18:47:17 -0500
From:	James Bottomley <James.Bottomley@...e.de>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Arve Hjønnevåg <arve@...roid.com>,
	tytso@....edu, LKML <linux-kernel@...r.kernel.org>,
	Florian Mickler <florian@...kler.org>,
	Linux PM <linux-pm@...ts.linux-foundation.org>,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	felipe.balbi@...ia.com, Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [linux-pm] [PATCH 0/8] Suspend block api (version 8)

On Mon, 2010-05-31 at 23:41 +0200, Thomas Gleixner wrote:
> On Sat, 29 May 2010, James Bottomley wrote:
> > On Sat, 2010-05-29 at 10:10 +0200, Peter Zijlstra wrote:
> > > 
> > > Not using suspend is exactly the point. As Alan has argued, propagating
> > > suspend blockers up into all regions of userspace will take much longer
> > > than fixing the hardware.
> > 
> > Strange, that's not what I heard as the possible solution.  I thought he
> > was advocating expressing the kernel side of suspend blockers as QoS
> > constraints.  Once we have QoS constraints correctly done in the kernel,
> > userspace still has to express its requirements.  If the requirements
> > are static, then they can be done from policy files or in some other
> > static way but if they're dynamic, they'll still have to be in the
> > applications ... in about the same places the android wakelocks are.
> 
> That's wrong. You only need the explicit dynamic QoS constraints for
> applications which follow the scheme:
> 
>      while (1) {
>      	   if (event_available())
> 	      process_event();
> 	   else
> 	      do_useless_crap_which_consumes_power();
>      }	   
> 
> which need the following annotation:
> 
>      while (1) {
>      	   block_suspend();
>      	   if (event_available()) {
> 	      process_event();
> 	      unblock_suspend();
> 	   } else {
> 	      unblock_suspend();
> 	      do_useless_crap_which_consumes_power();
>            }
>      }	   
> 
> Plus the kernel counterpart of drivers which take the suspend blocker
> in the interrupt handler and release it when the event queue is empty.
> 
> So that's done for making polling event handling power "efficient".
> 
> Even worse, you need the same "annotation" for non polling mode and it
> enforces the use of select() because you cannot take a suspend blocker
> across a blocking read() without adding more invasive interactions to
> the kernel..
> 
> So the "sane" app looks like:
> 
>    while (1) {
>    	 select();
> 	 block_suspend();
> 	 process_events();
> 	 unblock_suspend();
>    }
> 
> I'm really tired of arguing that this promotion of "programming style"
> is the worst idea ever, so let's look how you can do the same thing
> QoS based.
> 
> s/block_suspend()/qos(INTERACTIVE)/ and
> s/unblock_suspend()/qos(NONE)/ and
> s/block_magic()/qos_magic()/ in the drivers.

So this is the re-expression in terms of a QoS API that I mentioned ...
as I said, I think it's the way forwards. (from the android point of
view, it keeps the user space expression in exactly the same place as
the original wake locks, or suspend blocks, which is why it looks like a
rename to them).

> Yes, it's mostly the same, with a subtle difference:
> 
> While android can use it in the big hammer approach to disable the
> existing user initiated suspend via /sys/power/state, the rest of the
> world can benefit as well in various ways.
> 
>  - Sane applications which use a blocking event wait can be handled
>    with a static QoS setting simply because a blocking read relies on
>    the QoS state of the underlying I/O system.
> 
>  - Idle based suspend as the logical consequence of idle states is
>    just a matter of QoS constraint based decisions.
> 
>  - Untrusted apps can be confined in cgroups. The groups are set to
>    QoS(None) when user land decides that it's time to safe power
>    (e.g. screen lock kicks in)
> 
>  - QoS states can block applications from I/O when the I/O system is
>    set to a state which is exclusive.
> 
>   - ...

I understand this ... it's effectively the alan stern approach.  I've
already said that looks like the way forwards.

> So that allows to use the same mechanism for more than the android
> sledge hammer approach and confines the controversial use cases into
> android specific files without adding a hard to maintain user space
> interface which would prevent or at least make it hard to do some of
> the above mentioned things which we want to see implemented.

Yes, which is why I think something like this can be made to work ... I
don't really see that we differ on the broad brush picture.  As long as
the acceptable implementation accomplishes what everyone wants, I think
we're home.

James

> While I personally disagree with the android approach, I REALLY want
> to provide them a mechanism to make it work, but not for the price
> that it actively prevents solutions which are considered to be
> technically superior by a relevant group of developers/maintainers.
> 
> > > You got to realize this is about Linux as a whole, I really don't care
> > > one whit about the specific Android case. We want a solution that is
> > > generic enough to solve the power consumption problem and makes sense on
> > > future hardware.
> > 
> > I don't think anyone disagrees with this. As long as we find a long term
> > solution that satisfies the android case, everyone will be happy.
> 
> See above.
> 
> Thanks,
> 
> 	tglx


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