[PATCH/rfc] schedule /sys/device/.../power for removal

[PATCH/rfc] schedule /sys/device/.../power for removal

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

It is very ugly, and we really should use names instead.

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 421bcff..dfcfc47 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -6,6 +6,16 @@ be removed from this file.
 
 ---------------------------
 
+What:	/sys/device/.../power
+When:	July 2007
+Files:	
+Why:	Because it takes integers, and different userland applications
+	expect different numbers to mean different things.
+	(Pcmcia expect 2 for off, some other code expects 3 for off).
+Who:	Pavel Machek <pavel@suse.cz>
+
+---------------------------
+
 What:	devfs
 When:	July 2005
 Files:	fs/devfs/*, include/linux/devfs_fs*.h and assorted devfs

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Andrew Morton]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

Pavel Machek <pavel@ucw.cz> wrote:
>
> It is very ugly, and we really should use names instead.
> 
> Signed-off-by: Pavel Machek <pavel@suse.cz>
> 
> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index 421bcff..dfcfc47 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -6,6 +6,16 @@ be removed from this file.
>  
>  ---------------------------
>  
> +What:	/sys/device/.../power
> +When:	July 2007
> +Files:	
> +Why:	Because it takes integers, and different userland applications
> +	expect different numbers to mean different things.
> +	(Pcmcia expect 2 for off, some other code expects 3 for off).
> +Who:	Pavel Machek <pavel@suse.cz>
> +
> +---------------------------

What will be impacted by this?

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Pavel Machek]

On Pá 12-05-06 03:11:51, Andrew Morton wrote:
> Pavel Machek <pavel@ucw.cz> wrote:
> >
> > It is very ugly, and we really should use names instead.
> > 
> > Signed-off-by: Pavel Machek <pavel@suse.cz>
> > 
> > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > index 421bcff..dfcfc47 100644
> > --- a/Documentation/feature-removal-schedule.txt
> > +++ b/Documentation/feature-removal-schedule.txt
> > @@ -6,6 +6,16 @@ be removed from this file.
> >  
> >  ---------------------------
> >  
> > +What:	/sys/device/.../power
> > +When:	July 2007
> > +Files:	
> > +Why:	Because it takes integers, and different userland applications
> > +	expect different numbers to mean different things.
> > +	(Pcmcia expect 2 for off, some other code expects 3 for off).
> > +Who:	Pavel Machek <pavel@suse.cz>
> > +
> > +---------------------------
> 
> What will be impacted by this?

Some obscure place PCMCIA utils, IIRC. There was one more user, but I
do not remember who it was. Plus there may be few people doing echo
manually.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Andrew Morton]

Pavel Machek <pavel@suse.cz> wrote:
>
> On Pá 12-05-06 03:11:51, Andrew Morton wrote:
> > Pavel Machek <pavel@ucw.cz> wrote:
> > >
> > > It is very ugly, and we really should use names instead.
> > > 
> > > Signed-off-by: Pavel Machek <pavel@suse.cz>
> > > 
> > > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> > > index 421bcff..dfcfc47 100644
> > > --- a/Documentation/feature-removal-schedule.txt
> > > +++ b/Documentation/feature-removal-schedule.txt
> > > @@ -6,6 +6,16 @@ be removed from this file.
> > >  
> > >  ---------------------------
> > >  
> > > +What:	/sys/device/.../power
> > > +When:	July 2007
> > > +Files:	
> > > +Why:	Because it takes integers, and different userland applications
> > > +	expect different numbers to mean different things.
> > > +	(Pcmcia expect 2 for off, some other code expects 3 for off).
> > > +Who:	Pavel Machek <pavel@suse.cz>
> > > +
> > > +---------------------------
> > 
> > What will be impacted by this?
> 
> Some obscure place PCMCIA utils, IIRC. There was one more user, but I
> do not remember who it was. Plus there may be few people doing echo
> manually.

What will it be replaced with, and how will we communicate the need to
migrate to the various application developers?  We can't just rip it out
next year and point at some obscure entry in a kernel file and say "but we
told you".

Some five-times-per-reboot printk which tells people they should be using
/sys/device/.../whatever-it-will-be-called might be appropriate.

Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

Hi!

> > > > index 421bcff..dfcfc47 100644
> > > > --- a/Documentation/feature-removal-schedule.txt
> > > > +++ b/Documentation/feature-removal-schedule.txt
> > > > @@ -6,6 +6,16 @@ be removed from this file.
> > > >  
> > > >  ---------------------------
> > > >  
> > > > +What:	/sys/device/.../power
> > > > +When:	July 2007
> > > > +Files:	
> > > > +Why:	Because it takes integers, and different userland applications
> > > > +	expect different numbers to mean different things.
> > > > +	(Pcmcia expect 2 for off, some other code expects 3 for off).
> > > > +Who:	Pavel Machek <pavel@suse.cz>
> > > > +
> > > > +---------------------------
> > > 
> > > What will be impacted by this?
> > 
> > Some obscure place PCMCIA utils, IIRC. There was one more user, but I
> > do not remember who it was. Plus there may be few people doing echo
> > manually.
> 
> What will it be replaced with, and how will we communicate the need to
> migrate to the various application developers?  We can't just rip it out
> next year and point at some obscure entry in a kernel file and say "but we
> told you".

Ok, we do not have replacement ready, yet. Would it be feasible to
include warning now so that people are warned as early as possible,
while we are working on replacement? Ok, maybe not...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [linux-pm] Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[David Brownell]

On Friday 12 May 2006 3:11 am, Andrew Morton wrote:
> 
> What will be impacted by this?

Driver suspend/resume testing ... impact is strongly negative.

Without this interface, there is NO way to test individual drivers for
correct handling of suspend/resume calls; the only way to test drivers
is to suspend/resume the whole system, along with all other drivers in
the system.  Which means that ALL the drivers must work sanely before
tests for any one of them can succeed ... a losing model when you're
testing PM on new platforms.

Which IMO makes removing this a Bad Thing.  It needs to have some
kind of replacement in place before the "magic numbers" go away.

(The magic numbers are bad, and should go away -- yes.  Nobody has
really shown that userspace needs this mechanism for any purpose
other than driver testing.  Userspace device-specific power management
tools would need knowledge that's not yet exposed though sysfs.)

I think both Patrick Mochel and Alan Stern have sent patches at
various times to let individual drivers provide a list of named
states they support,  In some cases (like PCI) those lists could
be delegated to bus-specific code.

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Benjamin Herrenschmidt]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

On Fri, 2006-05-12 at 06:52 -0700, David Brownell wrote:
> On Friday 12 May 2006 3:11 am, Andrew Morton wrote:
> > 
> > What will be impacted by this?
> 
> Driver suspend/resume testing ... impact is strongly negative.
> 
> Without this interface, there is NO way to test individual drivers for
> correct handling of suspend/resume calls; the only way to test drivers
> is to suspend/resume the whole system, along with all other drivers in
> the system.  Which means that ALL the drivers must work sanely before
> tests for any one of them can succeed ... a losing model when you're
> testing PM on new platforms.
> 
> Which IMO makes removing this a Bad Thing.  It needs to have some
> kind of replacement in place before the "magic numbers" go away.

And that's why Pavel is not proposing to remove it right away... but to
schedule it's removal so that developpers know right now that building a
whole new kernel<->user interface based on that is not the smartest
thing to do.

> (The magic numbers are bad, and should go away -- yes.  Nobody has
> really shown that userspace needs this mechanism for any purpose
> other than driver testing.  Userspace device-specific power management
> tools would need knowledge that's not yet exposed though sysfs.)
> 
> I think both Patrick Mochel and Alan Stern have sent patches at
> various times to let individual drivers provide a list of named
> states they support,  In some cases (like PCI) those lists could
> be delegated to bus-specific code.
> 
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.osdl.org
> https://lists.osdl.org/mailman/listinfo/linux-pm


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

On Saturday 13 May 2006 5:13 pm, Benjamin Herrenschmidt wrote:
> On Fri, 2006-05-12 at 06:52 -0700, David Brownell wrote:
> > On Friday 12 May 2006 3:11 am, Andrew Morton wrote:
> > > 
> > > What will be impacted by this?
> > 
> > Driver suspend/resume testing ... impact is strongly negative.
> > ...
> > Which IMO makes removing this a Bad Thing.  It needs to have some
> > kind of replacement in place before the "magic numbers" go away.
> 
> And that's why Pavel is not proposing to remove it right away... but to
> schedule it's removal so that developpers know right now that building a
> whole new kernel<->user interface based on that is not the smartest
> thing to do.

How could we schedule the removal before we have even had a couple
releases to fine-tune its replacement, and verify that the main issues
with the current thing are fully resolved?

... plus, removing the whole power/* directory is clearly wrong.  The
issue that's been acknowledged is only with the contents of a single
file, power/state, not the whole directory.

There may be a bit of a gap in the process here.  "July 2007" is a
date that's not backed up by anything more than agreement that the
current approach is a lose.  Deprecation is not the same as removal.



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Ne 14-05-06 08:51:26, David Brownell wrote:
> On Saturday 13 May 2006 5:13 pm, Benjamin Herrenschmidt wrote:
> > On Fri, 2006-05-12 at 06:52 -0700, David Brownell wrote:
> > > On Friday 12 May 2006 3:11 am, Andrew Morton wrote:
> > > > 
> > > > What will be impacted by this?
> > > 
> > > Driver suspend/resume testing ... impact is strongly negative.
> > > ...
> > > Which IMO makes removing this a Bad Thing.  It needs to have some
> > > kind of replacement in place before the "magic numbers" go away.
> > 
> > And that's why Pavel is not proposing to remove it right away... but to
> > schedule it's removal so that developpers know right now that building a
> > whole new kernel<->user interface based on that is not the smartest
> > thing to do.
> 
> How could we schedule the removal before we have even had a couple
> releases to fine-tune its replacement, and verify that the main issues
> with the current thing are fully resolved?
> 
> ... plus, removing the whole power/* directory is clearly wrong.  The
> issue that's been acknowledged is only with the contents of a single
> file, power/state, not the whole directory.

Sorry, I meant only ../state file. Fixed locally.

> There may be a bit of a gap in the process here.  "July 2007" is a
> date that's not backed up by anything more than agreement that the
> current approach is a lose.  Deprecation is not the same as removal.

Maybe date will need to be shifted...
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On Sunday 14 May 2006 9:22 am, Pavel Machek wrote:

> > There may be a bit of a gap in the process here.  "July 2007" is a
> > date that's not backed up by anything more than agreement that the
> > current approach is a lose.  Deprecation is not the same as removal.
> 
> Maybe date will need to be shifted...

How about "one year after its replacement is ready"?

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Benjamin Herrenschmidt]

[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]


> I think both Patrick Mochel and Alan Stern have sent patches at
> various times to let individual drivers provide a list of named
> states they support,  In some cases (like PCI) those lists could
> be delegated to bus-specific code.

I've several times expressed my opinion there that it's not bus states
that should be exposed by a driver but the actual states that the
driver/device combination supports _regardless_ of what bus state they
map to (if any). Bus states simply mean nothing at this point (Unless
you are the bus driver). Of course we need both top-down and bottom-up
dependency mecanisms to handle state changes, but at the user level, a
given PCI driver shouldn't expose something like "D1", "D2" and "D3"...
those are PCI states that don't have a well defined semantics (and may
not be all supported by the device/driver). However, it's whatever
functional states that device/driver supports that shall be exposed.
Those can be "idle" and "suspended" for example, or there could be
several levels of "suspended".

Now of course, there is the problem that while such descriptive names
might have a sense to a user (and even then, only in one language,
english) they aren't very useful to some automated power management
mecanism.

That's the whole problem that needs solving, possibly by exposing as
much as the state dependencies as possible, along eventually with
informations such as can the device automatically trigger a transition
out of this state, eventually informations relative to max power
consumed in this state (not only embedded devices but also desktop
machines nowadays have fairly strick power budgets when entering global
system suspend), etc...

That was one of the goal of the mecanisms I described more than a year
ago with bit masks exposing bus state <-> device state dependencies,
though not a complete solution, it was more like a starting point to
think from.

Due to various personal matter I haven't followed up much on what came
next on this list or during the last PM summit, but from some of the
mails I'm reading, I have the feeling that there haven't been much
progress...

Ben
 


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]

On Saturday 13 May 2006 5:21 pm, Benjamin Herrenschmidt wrote:
> 
> > I think both Patrick Mochel and Alan Stern have sent patches at
> > various times to let individual drivers provide a list of named
> > states they support,  In some cases (like PCI) those lists could
> > be delegated to bus-specific code.
> 
> I've several times expressed my opinion there that it's not bus states
> that should be exposed by a driver but the actual states that the
> driver/device combination supports _regardless_ of what bus state they
> map to (if any). Bus states simply mean nothing at this point (Unless
> you are the bus driver).

I mostly agree ... you can have a pm-stupid driver for a given device,
or a very smart one, and they might need to expose different PM models
through any /sys/devices/.../power/state type files.

My disagreement is with the assumption that none of those states
will ever map directly to bus states.  (For those busses which define
them; not all do, but PCI does.)  Or perhaps with the assumption
that the driver must not name its states after the bus states they
use, even in common scenarios without one-to-many confusions.


> Of course we need both top-down and bottom-up 
> dependency mecanisms to handle state changes, but at the user level, a
> given PCI driver shouldn't expose something like "D1", "D2" and "D3"...
> those are PCI states that don't have a well defined semantics (and may
> not be all supported by the device/driver). 

Well certainly if the device doesn't implement PCI_D1 in hardware, then
that name shouldn't be used for a driver state.  But if it has only one
driver state that uses PCI_D2, why rule out calling that state PCI_D2?
Lots of PCI drivers don't even try to subdivide the PCI states, so every
driver state directly corresponds to one PCI state.

The PCI state semantics are well enough defined, but as I recall you want
device-specific details going beyond what the spec covers.  That is a
rather different issue.


> However, it's whatever 
> functional states that device/driver supports that shall be exposed.
> Those can be "idle" and "suspended" for example, or there could be
> several levels of "suspended".

And it would be less confusing to name those "suspended" levels with
names that make sense on multiple levels, like "PCI_D2" etc, than to
needlessly proliferate other names for those states.


> Now of course, there is the problem that while such descriptive names
> might have a sense to a user (and even then, only in one language,
> english) they aren't very useful to some automated power management
> mecanism.

Depends on the mechanism, surely.  And "PCI_D2" is already in that
language-neutral "doesn't get translation" category anyway.  Regardless,
userspace tools should just compare tokens.

 
> That's the whole problem that needs solving, possibly by exposing as
> much as the state dependencies as possible, along eventually with
> informations such as can the device automatically trigger a transition
> out of this state, eventually informations relative to max power
> consumed in this state 

I consider all those issues orthogonal to the issues addressed by
the /sys/devices/.../power/state files:  (a) reporting the power
state the driver is currently maintaining, and (b) changing that
to another state supported by that driver.

- Dave

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [PATCH/rfc] schedule /sys/device/.../power for removal

[Benjamin Herrenschmidt]

[-- Attachment #1: Type: text/plain, Size: 4089 bytes --]


> My disagreement is with the assumption that none of those states
> will ever map directly to bus states.  (For those busses which define
> them; not all do, but PCI does.)  Or perhaps with the assumption
> that the driver must not name its states after the bus states they
> use, even in common scenarios without one-to-many confusions.

I'm not saying hey won't map on bus states ever... I'm saying they
sometimes will or not, it's not a 1:1 relationchip, thus they are not
_directly_ related. This is why there is the whole issue of describing
the power state dependency that I talked about later in my mail.

> Well certainly if the device doesn't implement PCI_D1 in hardware, then
> that name shouldn't be used for a driver state.  But if it has only one
> driver state that uses PCI_D2, why rule out calling that state PCI_D2?

Because "PCI_D2" Means nothing from a device function point of view
imho. "Clock stopped" would make much more sense (provided it's what the
device does when in D2 mode, there is a definitive lack of consistency
three as D states have never been properly defined except for D3 cold :)

> Lots of PCI drivers don't even try to subdivide the PCI states, so every
> driver state directly corresponds to one PCI state.

Which is mostly bogus, and probably due to driver writers not even
bothering looking at what the device is actually doing when put into a
given Dx state... I suppose a lot of dumb devices also only really
handle D0 and some kind of Dx (where X can be 1...3 and that are about
equivalent).

> The PCI state semantics are well enough defined

No they are not. Historically, they were pretty much undefined.
Recently, Intel/PCISIG updated the PCI PM specification (that I suspect
pretty much no device vendor read) that provides _some_ clues as to what
states might actually mean, mostly that their semantic is essentially
specific to a given class (which is as good as being undefined as far as
the generic code is concerned). The only "sure" thing is that D0 is full
on and D3 is the max power management... In addition, there is this
optional mecanism for a device to report it's power consumption in the
various states, but that's pretty much it.

The only thing that has (finally) been properly defined are the bus
states (B0...B3).

Thus I still think that the PCI D state is an implementation detail to
be known by the driver only and doesn't have anything to do with a user
interface.

> , but as I recall you want
> device-specific details going beyond what the spec covers.  That is a
> rather different issue.

No, I want some reasonably defined semantics.

> > However, it's whatever 
> > functional states that device/driver supports that shall be exposed.
> > Those can be "idle" and "suspended" for example, or there could be
> > several levels of "suspended".
> 
> And it would be less confusing to name those "suspended" levels with
> names that make sense on multiple levels, like "PCI_D2" etc, than to
> needlessly proliferate other names for those states.

I disagree there....

> > Now of course, there is the problem that while such descriptive names
> > might have a sense to a user (and even then, only in one language,
> > english) they aren't very useful to some automated power management
> > mecanism.
> 
> Depends on the mechanism, surely.  And "PCI_D2" is already in that
> language-neutral "doesn't get translation" category anyway.  Regardless,
> userspace tools should just compare tokens.

etc...

> > That's the whole problem that needs solving, possibly by exposing as
> > much as the state dependencies as possible, along eventually with
> > informations such as can the device automatically trigger a transition
> > out of this state, eventually informations relative to max power
> > consumed in this state 
> 
> I consider all those issues orthogonal to the issues addressed by
> the /sys/devices/.../power/state files:  (a) reporting the power
> state the driver is currently maintaining, and (b) changing that
> to another state supported by that driver.

And handling the dependencies...

Ben.



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]