Re: Changes to PM layer break userspace
On Thursday 28 December 2006 5:31 am, Alan wrote: > > Seems to me anyone really desperate to put PCI devices into a low > > power mode, without driver support at the "ifdown" level, would be > > able just "rmmod driver; setpci". > > Incorrect for very obvious reasons - there may be two devices driven by > the same driver one up and one down. Let me emphasize "desperate". ;) The examples given were all cases where that didn't seem to be an issue. But agreed, the best approach is really to make devices not in active use (i.e. before "ifup", after "ifdown" ... maybe even whenever no driver is bound to the device) stay in low power states. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Thu, 2006-12-28 at 13:31 +, Alan wrote: > > Seems to me anyone really desperate to put PCI devices into a low > > power mode, without driver support at the "ifdown" level, would be > > able just "rmmod driver; setpci". > > Incorrect for very obvious reasons - there may be two devices driven by > the same driver one up and one down. btw this same "incorrect" applies to the sysfs method, that also does a more or less uncontrolled/uncoordinated power state switch. All the more reason to have the "normal" device interfaces do the right thing, so that the kernel has a standing chance to coordinate it properly. -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
> Seems to me anyone really desperate to put PCI devices into a low > power mode, without driver support at the "ifdown" level, would be > able just "rmmod driver; setpci". Incorrect for very obvious reasons - there may be two devices driven by the same driver one up and one down. Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Thursday 28 December 2006 5:31 am, Alan wrote: Seems to me anyone really desperate to put PCI devices into a low power mode, without driver support at the ifdown level, would be able just rmmod driver; setpci. Incorrect for very obvious reasons - there may be two devices driven by the same driver one up and one down. Let me emphasize desperate. ;) The examples given were all cases where that didn't seem to be an issue. But agreed, the best approach is really to make devices not in active use (i.e. before ifup, after ifdown ... maybe even whenever no driver is bound to the device) stay in low power states. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Seems to me anyone really desperate to put PCI devices into a low power mode, without driver support at the ifdown level, would be able just rmmod driver; setpci. Incorrect for very obvious reasons - there may be two devices driven by the same driver one up and one down. Alan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Thu, 2006-12-28 at 13:31 +, Alan wrote: Seems to me anyone really desperate to put PCI devices into a low power mode, without driver support at the ifdown level, would be able just rmmod driver; setpci. Incorrect for very obvious reasons - there may be two devices driven by the same driver one up and one down. btw this same incorrect applies to the sysfs method, that also does a more or less uncontrolled/uncoordinated power state switch. All the more reason to have the normal device interfaces do the right thing, so that the kernel has a standing chance to coordinate it properly. -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Friday 22 December 2006 1:09 pm, Pavel Machek wrote: > Actually, if we noticed power/state during PM framework review, it > would have been killed. It is just way too ugly. > > > > > In contrast, the /sys/devices/.../power/state API has never had many > > > > users beyond developers trying to test their drivers ... > > > > > > It's used on every Ubuntu and Suse system, > > > > Odd how the relevant Suse developers didn't mention any issues with > > those files going away, any of the times problems with them were > > discussed on the PM list. Also, I have a Suse system that doesn't > > use those files for anything ... maybe only newer release use it. > > Not on *every* suse system. power/state is known to oops kernels, so > it is only enabled when user explicitely asks for 'dangerous aggresive > experimental power saving' or something like that. So exactly what tool on Ubuntu uses this? Without any "dangerous! aggressive! experimental!" read-lights-siren-alarms-ringing alert level? Seems to me anyone really desperate to put PCI devices into a low power mode, without driver support at the "ifdown" level, would be able just "rmmod driver; setpci". Without risking software bugs. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Hi! > > That's a workable approach to resolving the underlying problem in the > > long term. In the short term, notice the system still works correctly > > if you don't try writing those files. > > Well, except I'm now burning an extra couple of watts of power. I > consider that pretty broken. Couple of watts is not that bad, considering usb still eats 4W more than it should. > > I'd not be keen on reverting Linus' patch [1] myself, even though few > > drivers have started to use that mechanism yet; that would be a step > > backwards, and would perpetuate users of that broken sysfs file. > > I'm sorry, which bit of "Don't break userspace API without adequate > prior warning and with a workable replacement" is difficult to > understand? It should not break any userspace... but you do not get the power savings any more. Sorry. This kind of powersaving is not available on recent kernels. Right fix is to extend wifi stack... and have ifconfig wlan0 powerdown, or something like that. Pavel -- Thanks for all the (sleeping) penguins. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Hi! > > The existence of the power/state interface wasn't a bug - it was a > > deliberate decision to add it. It's the only reason the > > dpm_runtime_suspend() interface exists. Actually, if we noticed power/state during PM framework review, it would have been killed. It is just way too ugly. > > > In contrast, the /sys/devices/.../power/state API has never had many > > > users beyond developers trying to test their drivers (without taking > > > the whole system into a low power state, which probably didn't work > > > in any case), and has *always* been problematic. And the change you > > > object to doesn't "break" anything fundamental, either. Everything > > > still works. > > > > It's used on every Ubuntu and Suse system, > > Odd how the relevant Suse developers didn't mention any issues with > those files going away, any of the times problems with them were > discussed on the PM list. Also, I have a Suse system that doesn't > use those files for anything ... maybe only newer release use it. Not on *every* suse system. power/state is known to oops kernels, so it is only enabled when user explicitely asks for 'dangerous aggresive experimental power saving' or something like that. -- Thanks for all the (sleeping) penguins. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Hi! The existence of the power/state interface wasn't a bug - it was a deliberate decision to add it. It's the only reason the dpm_runtime_suspend() interface exists. Actually, if we noticed power/state during PM framework review, it would have been killed. It is just way too ugly. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't break anything fundamental, either. Everything still works. It's used on every Ubuntu and Suse system, Odd how the relevant Suse developers didn't mention any issues with those files going away, any of the times problems with them were discussed on the PM list. Also, I have a Suse system that doesn't use those files for anything ... maybe only newer release use it. Not on *every* suse system. power/state is known to oops kernels, so it is only enabled when user explicitely asks for 'dangerous aggresive experimental power saving' or something like that. -- Thanks for all the (sleeping) penguins. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Hi! That's a workable approach to resolving the underlying problem in the long term. In the short term, notice the system still works correctly if you don't try writing those files. Well, except I'm now burning an extra couple of watts of power. I consider that pretty broken. Couple of watts is not that bad, considering usb still eats 4W more than it should. I'd not be keen on reverting Linus' patch [1] myself, even though few drivers have started to use that mechanism yet; that would be a step backwards, and would perpetuate users of that broken sysfs file. I'm sorry, which bit of Don't break userspace API without adequate prior warning and with a workable replacement is difficult to understand? It should not break any userspace... but you do not get the power savings any more. Sorry. This kind of powersaving is not available on recent kernels. Right fix is to extend wifi stack... and have ifconfig wlan0 powerdown, or something like that. Pavel -- Thanks for all the (sleeping) penguins. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Friday 22 December 2006 1:09 pm, Pavel Machek wrote: Actually, if we noticed power/state during PM framework review, it would have been killed. It is just way too ugly. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers ... It's used on every Ubuntu and Suse system, Odd how the relevant Suse developers didn't mention any issues with those files going away, any of the times problems with them were discussed on the PM list. Also, I have a Suse system that doesn't use those files for anything ... maybe only newer release use it. Not on *every* suse system. power/state is known to oops kernels, so it is only enabled when user explicitely asks for 'dangerous aggresive experimental power saving' or something like that. So exactly what tool on Ubuntu uses this? Without any dangerous! aggressive! experimental! read-lights-siren-alarms-ringing alert level? Seems to me anyone really desperate to put PCI devices into a low power mode, without driver support at the ifdown level, would be able just rmmod driver; setpci. Without risking software bugs. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
> Seriously. How many pieces of userspace-visible functionality have > recently been removed without there being any sort of alternative? There IS an alternative, you're using it for networking: You *down the interface*. If there's a NIC that doesn't support that let us (or preferably netdev) know and it'll get fixed quickly I'm sure. -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 09:34:17PM -0800, Greg KH wrote: > I would be very interested to see any newer SuSE programs using that > interface. Just point them out to me and I'll quickly fix them. As far as I can tell, powersaved still uses these.. I'm not quite sure how you can fix it without just removing the functionality from it... > And yes, as a SuSE developer (and one of the people in charge of the > SuSE kernels), I have no problem with these files just going away. > Because, as David keeps repeating, they are broken and wrong. In the common case, it works perfectly well for the management of individual PCI devices. Yes it's "wrong", in much the same way as (say) the IDE bus registration/unregistration code. But we keep that around because despite it being even more broken than devices/.../power/state, people are still actually using it and we haven't provided any sort of alternative. Seriously. How many pieces of userspace-visible functionality have recently been removed without there being any sort of alternative? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 09:14:49PM -0800, David Brownell wrote: > On Tuesday 19 December 2006 8:26 pm, Matthew Garrett wrote: > > On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: > > It's perfectly reasonable to > > refer to it as a flawed interface, or perhaps even a buggy one. But in > > itself, it's clearly not a bug. > > This class of bug is also called a "design bug" or sometimes "mistake". Exactly, those "power" files actually pre-date the actual tree of devices itself. They were just holders for what the original developer thought was going to be needed, but was never properly implemented due to some job changes (note, this was not myself...) > > > In contrast, the /sys/devices/.../power/state API has never had many > > > users beyond developers trying to test their drivers (without taking > > > the whole system into a low power state, which probably didn't work > > > in any case), and has *always* been problematic. And the change you > > > object to doesn't "break" anything fundamental, either. Everything > > > still works. > > > > It's used on every Ubuntu and Suse system, > > Odd how the relevant Suse developers didn't mention any issues with > those files going away, any of the times problems with them were > discussed on the PM list. Also, I have a Suse system that doesn't > use those files for anything ... maybe only newer release use it. I would be very interested to see any newer SuSE programs using that interface. Just point them out to me and I'll quickly fix them. And yes, as a SuSE developer (and one of the people in charge of the SuSE kernels), I have no problem with these files just going away. Because, as David keeps repeating, they are broken and wrong. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 8:26 pm, Matthew Garrett wrote: > On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: > The existence of the power/state interface wasn't a bug - it was a > deliberate decision to add it. It's the only reason the > dpm_runtime_suspend() interface exists. All that buggy infrastructure talks together, yes. Those dpm_*() calls are in the same "will remove" task item. > It's perfectly reasonable to > refer to it as a flawed interface, or perhaps even a buggy one. But in > itself, it's clearly not a bug. This class of bug is also called a "design bug" or sometimes "mistake". > > In contrast, the /sys/devices/.../power/state API has never had many > > users beyond developers trying to test their drivers (without taking > > the whole system into a low power state, which probably didn't work > > in any case), and has *always* been problematic. And the change you > > object to doesn't "break" anything fundamental, either. Everything > > still works. > > It's used on every Ubuntu and Suse system, Odd how the relevant Suse developers didn't mention any issues with those files going away, any of the times problems with them were discussed on the PM list. Also, I have a Suse system that doesn't use those files for anything ... maybe only newer release use it. I've got some Ubuntu going too, which hasn't (visibly) suffered from any of these changes. - dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: > On Tuesday 19 December 2006 4:25 pm, Matthew Garrett wrote: > > 1) feature-removal-schedule.txt says that it'll be removed in July 2007. > > This isn't July 2007. > > Which is why the functionality is still there. Merely broken in the majority of cases... > > 2) The functionality was disabled in 2.6.19. The addition to > > feature-removal-schedule.txt was in, uh, 2.6.19. > > Please respond to the technical explanation I provided, and stop > referring to the functionality ** which is still there and works ** > as being disabled. The breakage is that devices that are happy to suspend with enabled interrupts can no longer be suspended from userspace. Refusing to suspend a single device on the basis that some other driver on the bus may, potentially, at some point require some suspend code to be run with disabled interrupts is not a sensible choice. Especially since I can't actually find a single driver in the kernel tree that currently uses this functionality. > I can't help it if that schedule.txt patch took until 2.6.19 to get > upstream; ISTR it was available before 2.6.18 shipped. Maybe patches > to that file should be accelerated, even into the stable series. That would still not have provided anywhere near enough warning. > One of the missing steps in Linus' formulation there is that not all > interfaces are equivalent in terms of support guarantee. Bugs are > interfaces, for example, and sometimes folk wrongly depend on them > when they persist for a long time (like, cough, this one). The existence of the power/state interface wasn't a bug - it was a deliberate decision to add it. It's the only reason the dpm_runtime_suspend() interface exists. It's perfectly reasonable to refer to it as a flawed interface, or perhaps even a buggy one. But in itself, it's clearly not a bug. And it's perfectly reasonable for userland to depend on interfaces that are deliberately exposed by the kernel. > In contrast, the /sys/devices/.../power/state API has never had many > users beyond developers trying to test their drivers (without taking > the whole system into a low power state, which probably didn't work > in any case), and has *always* been problematic. And the change you > object to doesn't "break" anything fundamental, either. Everything > still works. It's used on every Ubuntu and Suse system, and the change means that certain functionality no longer works - it's now impossible to prevent my wireless hardware from drawing power when I'm not using it, for example. If the WE power operations were deliberately disabled, then that would also be a bug. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 7:43 pm, Matthew Garrett wrote: > > Do you have an alternate solution? > > How about something like this? Entirely untested, but I think it shows > the basic idea. Other than indentation/whitespace bugs, it seems to encapsulate the layering violation needed to get those deprecated files working again for PCI (and platform_bus). I'd rename the new bus method though; maybe "pm_has_noirq_stage()" or somesuch. Your name is so generic that it'd be a surprise if the answer were ever "no"! You should also list this new call in the feature-removal.txt entry for stuff that gets removed with /sys/devices/.../power/state files, since it's another mechanism that only exists to prop up that broken API, and should vanish at the same time that API does. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 4:25 pm, Matthew Garrett wrote: > On Tue, Dec 19, 2006 at 01:34:49PM -0800, David Brownell wrote: > > > Documentation/feature-removal-schedule.txt has warned about this since > > August, and the PM list has discussed how broken that model is numerous > > times over the past several years. (I'm pretty sure that discussion has > > leaked out to LKML on occasion.) It shouldn't be news today. > > 1) feature-removal-schedule.txt says that it'll be removed in July 2007. > This isn't July 2007. Which is why the functionality is still there. > 2) The functionality was disabled in 2.6.19. The addition to > feature-removal-schedule.txt was in, uh, 2.6.19. Please respond to the technical explanation I provided, and stop referring to the functionality ** which is still there and works ** as being disabled. The fact that PCI exposes a mechanism that conflicts with that is a separate issue. Whining does not help. I can't help it if that schedule.txt patch took until 2.6.19 to get upstream; ISTR it was available before 2.6.18 shipped. Maybe patches to that file should be accelerated, even into the stable series. > 3) "The whole _point_ of a kernel is to act as a abstraction layer and > resource management between user programs and hardware/outside world. > That's why kernels _exist_. Breaking user-land API's is thus by > definition something totally idiotic. > > If you need to break something, you create a new interface, and try to > translate between the two, and maybe you deprecate the old one so that > it can be removed once it's not in use any more. If you can't see that > this is how a kernel should work, you're missing the point of having a > kernel in the first place." > > Linus, http://lkml.org/lkml/2006/10/4/327 So I'm amused that the problem you refer to is the direct consequence of Linus' patch to add the suspend_late()/resume_early() mechanism into the PCI driver framework. (Again, see the technical explanation; and please try to have a technical discussion, not a flamefest.) One of the missing steps in Linus' formulation there is that not all interfaces are equivalent in terms of support guarantee. Bugs are interfaces, for example, and sometimes folk wrongly depend on them when they persist for a long time (like, cough, this one). His comment was specifically about breaking a widely used API that many people have been relying on since, oh, about 1996, and had been well proven in that time. And the change was a "system doesn't work" level change. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't "break" anything fundamental, either. Everything still works. In terms of any reasonable expectations about support, those two changes aren't comparable. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 07:19:36PM -0800, David Brownell wrote:
> On Tuesday 19 December 2006 4:09 pm, Matthew Garrett wrote:
> > I'm sorry, which bit of "Don't break userspace API without adequate
> > prior warning and with a workable replacement" is difficult to
> > understand?
>
> What part of "it was already broken" do YOU not understand? The
> whole notion is unsustainable. It doesn't work cross-platform, or
> for multiple bus types. It confuses system-wide suspend mechanisms
> with runtime mechanisms. It breaks guaranteed parent/child ordering
> of suspend/resume calls. (And more...)
Linux is utterly riddled with broken APIs. It's possible to see that as
a downside of the "Release early, release often" model, but the
advantage is that we get the opportunity to determine how these
interfaces are broken. Based on that, we can either improve the existing
interface or decide that it's broken beyond repair and design a new one.
What we don't do is decide that an interface is broken, deprecate it
and in the same release break it even for the cases where it
previously worked. That's just insane.
> Let us know when you get tired of whining and want to move on to
> getting a real solution to the set of problems here. I've pointed
> out that reverting Linus' patch would be one option to get your
> short term issue rsolved ... that would remove a capability from
> PCI drivers, but you could then use that deprecated mechanism.
> I've also pointed out that you could start working towards a real
> long term solution.
I could, and in the long run I intend to. On the other hand, I don't
expect to have enough time to fix every single in-tree network driver
before 2.6.20, so...
> Do you have an alternate solution?
How about something like this? Entirely untested, but I think it shows
the basic idea.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f9c903b..4865918 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -597,6 +597,17 @@ static int platform_resume(struct device * dev)
return ret;
}
+static int platform_requires_disabled_interrupts(struct device * dev)
+{
+ int ret = 0;
+
+ if (dev->driver && (dev->driver->resume_early
+ || dev->driver->suspend_late))
+ ret = 1;
+
+ return ret;
+}
+
struct bus_type platform_bus_type = {
.name = "platform",
.dev_attrs = platform_dev_attrs,
@@ -604,8 +615,9 @@ struct bus_type platform_bus_type = {
.uevent = platform_uevent,
.suspend= platform_suspend,
.suspend_late = platform_suspend_late,
- .resume_early = platform_resume_early,
+ .resume_early = platform_resume_early,
.resume = platform_resume,
+ .requires_disabled_interrupts = platform_requires_disabled_interrupts,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 2d47517..97c6d65 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -46,7 +46,8 @@ static ssize_t state_store(struct device * dev, struct
device_attribute *attr, c
int error = -EINVAL;
/* disallow incomplete suspend sequences */
- if (dev->bus && (dev->bus->suspend_late || dev->bus->resume_early))
+ if (dev->bus && dev->bus->requires_disabled_interrupts
+ && dev->bus->requries_disabled_interrupts())
return error;
state.event = PM_EVENT_SUSPEND;
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e5ae3a0..9808d42 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -351,6 +351,18 @@ static int pci_device_resume(struct device * dev)
return error;
}
+static int pci_device_requires_disabled_interrupts(struct device * dev)
+{
+ int error = 0;
+ struct pci_dev * pci_dev = to_pci_dev(dev);
+ struct pci_driver * drv = pci_dev->driver;
+
+ if (drv && (drv->resume_early || drv_suspend_late))
+ error = 1;
+
+ return error;
+}
+
static int pci_device_resume_early(struct device * dev)
{
int error = 0;
@@ -569,6 +581,7 @@ struct bus_type pci_bus_type = {
.suspend_late = pci_device_suspend_late,
.resume_early = pci_device_resume_early,
.resume = pci_device_resume,
+ .requires_disabled_interrupts = pci_requires_disabled_interrupts,
.shutdown = pci_device_shutdown,
.dev_attrs = pci_dev_attrs,
};
diff --git a/include/linux/device.h b/include/linux/device.h
index 49ab53c..0686234 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -59,6 +59,7 @@ struct bus_type {
int (*suspend)(struct device * dev, pm_message_t state);
int (*suspend_late)(struct device * dev, pm_message_t state);
int (*resume_early)(struct device * dev);
+ int
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 4:09 pm, Matthew Garrett wrote: > On Tue, Dec 19, 2006 at 03:36:28PM -0800, David Brownell wrote: > > On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: > > > The fact that something is scheduled to be removed in July 2007 does > > > *not* mean it's acceptable to break it in 2006. We need to find a way to > > > fix this functionality in the meantime. > > > > The disconnect here is analagous to: I tell you the alleged perpetual > > motion machine never worked, and can't ever work; and you push back and > > say that you need a perpetual motion machine that works, NOW please, > > because you need something that pushes those widgets around. (There are > > better ways to push widgets than side effects of a broken machine...) > > But it *did* work. Having been on the other side ... I can testify that if you think it actually worked, it's because you're ignoring all the nasty failure modes. > > I'd not be keen on reverting Linus' patch [1] myself, even though few > > drivers have started to use that mechanism yet; that would be a step > > backwards, and would perpetuate users of that broken sysfs file. > > I'm sorry, which bit of "Don't break userspace API without adequate > prior warning and with a workable replacement" is difficult to > understand? What part of "it was already broken" do YOU not understand? The whole notion is unsustainable. It doesn't work cross-platform, or for multiple bus types. It confuses system-wide suspend mechanisms with runtime mechanisms. It breaks guaranteed parent/child ordering of suspend/resume calls. (And more...) Let us know when you get tired of whining and want to move on to getting a real solution to the set of problems here. I've pointed out that reverting Linus' patch would be one option to get your short term issue rsolved ... that would remove a capability from PCI drivers, but you could then use that deprecated mechanism. I've also pointed out that you could start working towards a real long term solution. Do you have an alternate solution? - Dave - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 03:36:28PM -0800, David Brownell wrote: > On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: > > The fact that something is scheduled to be removed in July 2007 does > > *not* mean it's acceptable to break it in 2006. We need to find a way to > > fix this functionality in the meantime. > > The disconnect here is analagous to: I tell you the alleged perpetual > motion machine never worked, and can't ever work; and you push back and > say that you need a perpetual motion machine that works, NOW please, > because you need something that pushes those widgets around. (There are > better ways to push widgets than side effects of a broken machine...) But it *did* work. Userspace could ask the device to suspend, and (in general) that would result in the device going into a lower power state. You've broken that without providing an alternative. > Given that your examples are network adapters, you should really look > more at why "ifdown eth0" (etc) having drivers put the device into a > low power state (like PCI D3hot, or maybe D2) wouldn't work in any > particular case. If you actually have such cases, then maybe those > specific drivers need to drive new power management interfaces. We seem to be arguing at cross purposes here. I've absolutely no objection to this approach in the long run, just as I've got no objection to flying cars or food pills or moon pods. When these things exist, the world will indeed be a glorious place. But that doesn't justify me slashing your tyres, poisoning your crops or setting fire to whatever the real-world analogue of a moon pod is. I had something that worked. Now I don't, but instead have the promise that at some point I'll have something better. Understand why I'm a touch irritated? > That's a workable approach to resolving the underlying problem in the > long term. In the short term, notice the system still works correctly > if you don't try writing those files. Well, except I'm now burning an extra couple of watts of power. I consider that pretty broken. > I'd not be keen on reverting Linus' patch [1] myself, even though few > drivers have started to use that mechanism yet; that would be a step > backwards, and would perpetuate users of that broken sysfs file. I'm sorry, which bit of "Don't break userspace API without adequate prior warning and with a workable replacement" is difficult to understand? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: > On Tue, Dec 19, 2006 at 01:22:12PM -0800, David Brownell wrote: > > As a generic mechanism, that interface has *ALWAYS* been "broken > > by design"; I'd call it unfixable. It's deprecated, and scheduled > > to vanish; see Documentation/feature-removal-schedule.txt ... > > The fact that something is scheduled to be removed in July 2007 does > *not* mean it's acceptable to break it in 2006. We need to find a way to > fix this functionality in the meantime. The disconnect here is analagous to: I tell you the alleged perpetual motion machine never worked, and can't ever work; and you push back and say that you need a perpetual motion machine that works, NOW please, because you need something that pushes those widgets around. (There are better ways to push widgets than side effects of a broken machine...) Given that your examples are network adapters, you should really look more at why "ifdown eth0" (etc) having drivers put the device into a low power state (like PCI D3hot, or maybe D2) wouldn't work in any particular case. If you actually have such cases, then maybe those specific drivers need to drive new power management interfaces. That's a workable approach to resolving the underlying problem in the long term. In the short term, notice the system still works correctly if you don't try writing those files. I'd not be keen on reverting Linus' patch [1] myself, even though few drivers have started to use that mechanism yet; that would be a step backwards, and would perpetuate users of that broken sysfs file. - Dave [1] cbd69dbbf1adfce6e048f15afc8629901ca9dae5 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: On Tue, Dec 19, 2006 at 01:22:12PM -0800, David Brownell wrote: As a generic mechanism, that interface has *ALWAYS* been broken by design; I'd call it unfixable. It's deprecated, and scheduled to vanish; see Documentation/feature-removal-schedule.txt ... The fact that something is scheduled to be removed in July 2007 does *not* mean it's acceptable to break it in 2006. We need to find a way to fix this functionality in the meantime. The disconnect here is analagous to: I tell you the alleged perpetual motion machine never worked, and can't ever work; and you push back and say that you need a perpetual motion machine that works, NOW please, because you need something that pushes those widgets around. (There are better ways to push widgets than side effects of a broken machine...) Given that your examples are network adapters, you should really look more at why ifdown eth0 (etc) having drivers put the device into a low power state (like PCI D3hot, or maybe D2) wouldn't work in any particular case. If you actually have such cases, then maybe those specific drivers need to drive new power management interfaces. That's a workable approach to resolving the underlying problem in the long term. In the short term, notice the system still works correctly if you don't try writing those files. I'd not be keen on reverting Linus' patch [1] myself, even though few drivers have started to use that mechanism yet; that would be a step backwards, and would perpetuate users of that broken sysfs file. - Dave [1] cbd69dbbf1adfce6e048f15afc8629901ca9dae5 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 03:36:28PM -0800, David Brownell wrote: On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: The fact that something is scheduled to be removed in July 2007 does *not* mean it's acceptable to break it in 2006. We need to find a way to fix this functionality in the meantime. The disconnect here is analagous to: I tell you the alleged perpetual motion machine never worked, and can't ever work; and you push back and say that you need a perpetual motion machine that works, NOW please, because you need something that pushes those widgets around. (There are better ways to push widgets than side effects of a broken machine...) But it *did* work. Userspace could ask the device to suspend, and (in general) that would result in the device going into a lower power state. You've broken that without providing an alternative. Given that your examples are network adapters, you should really look more at why ifdown eth0 (etc) having drivers put the device into a low power state (like PCI D3hot, or maybe D2) wouldn't work in any particular case. If you actually have such cases, then maybe those specific drivers need to drive new power management interfaces. We seem to be arguing at cross purposes here. I've absolutely no objection to this approach in the long run, just as I've got no objection to flying cars or food pills or moon pods. When these things exist, the world will indeed be a glorious place. But that doesn't justify me slashing your tyres, poisoning your crops or setting fire to whatever the real-world analogue of a moon pod is. I had something that worked. Now I don't, but instead have the promise that at some point I'll have something better. Understand why I'm a touch irritated? That's a workable approach to resolving the underlying problem in the long term. In the short term, notice the system still works correctly if you don't try writing those files. Well, except I'm now burning an extra couple of watts of power. I consider that pretty broken. I'd not be keen on reverting Linus' patch [1] myself, even though few drivers have started to use that mechanism yet; that would be a step backwards, and would perpetuate users of that broken sysfs file. I'm sorry, which bit of Don't break userspace API without adequate prior warning and with a workable replacement is difficult to understand? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 4:09 pm, Matthew Garrett wrote: On Tue, Dec 19, 2006 at 03:36:28PM -0800, David Brownell wrote: On Tuesday 19 December 2006 2:57 pm, Matthew Garrett wrote: The fact that something is scheduled to be removed in July 2007 does *not* mean it's acceptable to break it in 2006. We need to find a way to fix this functionality in the meantime. The disconnect here is analagous to: I tell you the alleged perpetual motion machine never worked, and can't ever work; and you push back and say that you need a perpetual motion machine that works, NOW please, because you need something that pushes those widgets around. (There are better ways to push widgets than side effects of a broken machine...) But it *did* work. Having been on the other side ... I can testify that if you think it actually worked, it's because you're ignoring all the nasty failure modes. I'd not be keen on reverting Linus' patch [1] myself, even though few drivers have started to use that mechanism yet; that would be a step backwards, and would perpetuate users of that broken sysfs file. I'm sorry, which bit of Don't break userspace API without adequate prior warning and with a workable replacement is difficult to understand? What part of it was already broken do YOU not understand? The whole notion is unsustainable. It doesn't work cross-platform, or for multiple bus types. It confuses system-wide suspend mechanisms with runtime mechanisms. It breaks guaranteed parent/child ordering of suspend/resume calls. (And more...) Let us know when you get tired of whining and want to move on to getting a real solution to the set of problems here. I've pointed out that reverting Linus' patch would be one option to get your short term issue rsolved ... that would remove a capability from PCI drivers, but you could then use that deprecated mechanism. I've also pointed out that you could start working towards a real long term solution. Do you have an alternate solution? - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 07:19:36PM -0800, David Brownell wrote:
On Tuesday 19 December 2006 4:09 pm, Matthew Garrett wrote:
I'm sorry, which bit of Don't break userspace API without adequate
prior warning and with a workable replacement is difficult to
understand?
What part of it was already broken do YOU not understand? The
whole notion is unsustainable. It doesn't work cross-platform, or
for multiple bus types. It confuses system-wide suspend mechanisms
with runtime mechanisms. It breaks guaranteed parent/child ordering
of suspend/resume calls. (And more...)
Linux is utterly riddled with broken APIs. It's possible to see that as
a downside of the Release early, release often model, but the
advantage is that we get the opportunity to determine how these
interfaces are broken. Based on that, we can either improve the existing
interface or decide that it's broken beyond repair and design a new one.
What we don't do is decide that an interface is broken, deprecate it
and in the same release break it even for the cases where it
previously worked. That's just insane.
Let us know when you get tired of whining and want to move on to
getting a real solution to the set of problems here. I've pointed
out that reverting Linus' patch would be one option to get your
short term issue rsolved ... that would remove a capability from
PCI drivers, but you could then use that deprecated mechanism.
I've also pointed out that you could start working towards a real
long term solution.
I could, and in the long run I intend to. On the other hand, I don't
expect to have enough time to fix every single in-tree network driver
before 2.6.20, so...
Do you have an alternate solution?
How about something like this? Entirely untested, but I think it shows
the basic idea.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f9c903b..4865918 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -597,6 +597,17 @@ static int platform_resume(struct device * dev)
return ret;
}
+static int platform_requires_disabled_interrupts(struct device * dev)
+{
+ int ret = 0;
+
+ if (dev-driver (dev-driver-resume_early
+ || dev-driver-suspend_late))
+ ret = 1;
+
+ return ret;
+}
+
struct bus_type platform_bus_type = {
.name = platform,
.dev_attrs = platform_dev_attrs,
@@ -604,8 +615,9 @@ struct bus_type platform_bus_type = {
.uevent = platform_uevent,
.suspend= platform_suspend,
.suspend_late = platform_suspend_late,
- .resume_early = platform_resume_early,
+ .resume_early = platform_resume_early,
.resume = platform_resume,
+ .requires_disabled_interrupts = platform_requires_disabled_interrupts,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 2d47517..97c6d65 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -46,7 +46,8 @@ static ssize_t state_store(struct device * dev, struct
device_attribute *attr, c
int error = -EINVAL;
/* disallow incomplete suspend sequences */
- if (dev-bus (dev-bus-suspend_late || dev-bus-resume_early))
+ if (dev-bus dev-bus-requires_disabled_interrupts
+dev-bus-requries_disabled_interrupts())
return error;
state.event = PM_EVENT_SUSPEND;
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e5ae3a0..9808d42 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -351,6 +351,18 @@ static int pci_device_resume(struct device * dev)
return error;
}
+static int pci_device_requires_disabled_interrupts(struct device * dev)
+{
+ int error = 0;
+ struct pci_dev * pci_dev = to_pci_dev(dev);
+ struct pci_driver * drv = pci_dev-driver;
+
+ if (drv (drv-resume_early || drv_suspend_late))
+ error = 1;
+
+ return error;
+}
+
static int pci_device_resume_early(struct device * dev)
{
int error = 0;
@@ -569,6 +581,7 @@ struct bus_type pci_bus_type = {
.suspend_late = pci_device_suspend_late,
.resume_early = pci_device_resume_early,
.resume = pci_device_resume,
+ .requires_disabled_interrupts = pci_requires_disabled_interrupts,
.shutdown = pci_device_shutdown,
.dev_attrs = pci_dev_attrs,
};
diff --git a/include/linux/device.h b/include/linux/device.h
index 49ab53c..0686234 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -59,6 +59,7 @@ struct bus_type {
int (*suspend)(struct device * dev, pm_message_t state);
int (*suspend_late)(struct device * dev, pm_message_t state);
int (*resume_early)(struct device * dev);
+ int (*requires_disabled_interrupts)(struct device * dev);
int (*resume)(struct
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 4:25 pm, Matthew Garrett wrote: On Tue, Dec 19, 2006 at 01:34:49PM -0800, David Brownell wrote: Documentation/feature-removal-schedule.txt has warned about this since August, and the PM list has discussed how broken that model is numerous times over the past several years. (I'm pretty sure that discussion has leaked out to LKML on occasion.) It shouldn't be news today. 1) feature-removal-schedule.txt says that it'll be removed in July 2007. This isn't July 2007. Which is why the functionality is still there. 2) The functionality was disabled in 2.6.19. The addition to feature-removal-schedule.txt was in, uh, 2.6.19. Please respond to the technical explanation I provided, and stop referring to the functionality ** which is still there and works ** as being disabled. The fact that PCI exposes a mechanism that conflicts with that is a separate issue. Whining does not help. I can't help it if that schedule.txt patch took until 2.6.19 to get upstream; ISTR it was available before 2.6.18 shipped. Maybe patches to that file should be accelerated, even into the stable series. 3) The whole _point_ of a kernel is to act as a abstraction layer and resource management between user programs and hardware/outside world. That's why kernels _exist_. Breaking user-land API's is thus by definition something totally idiotic. If you need to break something, you create a new interface, and try to translate between the two, and maybe you deprecate the old one so that it can be removed once it's not in use any more. If you can't see that this is how a kernel should work, you're missing the point of having a kernel in the first place. Linus, http://lkml.org/lkml/2006/10/4/327 So I'm amused that the problem you refer to is the direct consequence of Linus' patch to add the suspend_late()/resume_early() mechanism into the PCI driver framework. (Again, see the technical explanation; and please try to have a technical discussion, not a flamefest.) One of the missing steps in Linus' formulation there is that not all interfaces are equivalent in terms of support guarantee. Bugs are interfaces, for example, and sometimes folk wrongly depend on them when they persist for a long time (like, cough, this one). His comment was specifically about breaking a widely used API that many people have been relying on since, oh, about 1996, and had been well proven in that time. And the change was a system doesn't work level change. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't break anything fundamental, either. Everything still works. In terms of any reasonable expectations about support, those two changes aren't comparable. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 7:43 pm, Matthew Garrett wrote: Do you have an alternate solution? How about something like this? Entirely untested, but I think it shows the basic idea. Other than indentation/whitespace bugs, it seems to encapsulate the layering violation needed to get those deprecated files working again for PCI (and platform_bus). I'd rename the new bus method though; maybe pm_has_noirq_stage() or somesuch. Your name is so generic that it'd be a surprise if the answer were ever no! You should also list this new call in the feature-removal.txt entry for stuff that gets removed with /sys/devices/.../power/state files, since it's another mechanism that only exists to prop up that broken API, and should vanish at the same time that API does. - Dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: On Tuesday 19 December 2006 4:25 pm, Matthew Garrett wrote: 1) feature-removal-schedule.txt says that it'll be removed in July 2007. This isn't July 2007. Which is why the functionality is still there. Merely broken in the majority of cases... 2) The functionality was disabled in 2.6.19. The addition to feature-removal-schedule.txt was in, uh, 2.6.19. Please respond to the technical explanation I provided, and stop referring to the functionality ** which is still there and works ** as being disabled. The breakage is that devices that are happy to suspend with enabled interrupts can no longer be suspended from userspace. Refusing to suspend a single device on the basis that some other driver on the bus may, potentially, at some point require some suspend code to be run with disabled interrupts is not a sensible choice. Especially since I can't actually find a single driver in the kernel tree that currently uses this functionality. I can't help it if that schedule.txt patch took until 2.6.19 to get upstream; ISTR it was available before 2.6.18 shipped. Maybe patches to that file should be accelerated, even into the stable series. That would still not have provided anywhere near enough warning. One of the missing steps in Linus' formulation there is that not all interfaces are equivalent in terms of support guarantee. Bugs are interfaces, for example, and sometimes folk wrongly depend on them when they persist for a long time (like, cough, this one). The existence of the power/state interface wasn't a bug - it was a deliberate decision to add it. It's the only reason the dpm_runtime_suspend() interface exists. It's perfectly reasonable to refer to it as a flawed interface, or perhaps even a buggy one. But in itself, it's clearly not a bug. And it's perfectly reasonable for userland to depend on interfaces that are deliberately exposed by the kernel. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't break anything fundamental, either. Everything still works. It's used on every Ubuntu and Suse system, and the change means that certain functionality no longer works - it's now impossible to prevent my wireless hardware from drawing power when I'm not using it, for example. If the WE power operations were deliberately disabled, then that would also be a bug. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tuesday 19 December 2006 8:26 pm, Matthew Garrett wrote: On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: The existence of the power/state interface wasn't a bug - it was a deliberate decision to add it. It's the only reason the dpm_runtime_suspend() interface exists. All that buggy infrastructure talks together, yes. Those dpm_*() calls are in the same will remove task item. It's perfectly reasonable to refer to it as a flawed interface, or perhaps even a buggy one. But in itself, it's clearly not a bug. This class of bug is also called a design bug or sometimes mistake. In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't break anything fundamental, either. Everything still works. It's used on every Ubuntu and Suse system, Odd how the relevant Suse developers didn't mention any issues with those files going away, any of the times problems with them were discussed on the PM list. Also, I have a Suse system that doesn't use those files for anything ... maybe only newer release use it. I've got some Ubuntu going too, which hasn't (visibly) suffered from any of these changes. - dave - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 09:14:49PM -0800, David Brownell wrote: On Tuesday 19 December 2006 8:26 pm, Matthew Garrett wrote: On Tue, Dec 19, 2006 at 07:59:42PM -0800, David Brownell wrote: It's perfectly reasonable to refer to it as a flawed interface, or perhaps even a buggy one. But in itself, it's clearly not a bug. This class of bug is also called a design bug or sometimes mistake. Exactly, those power files actually pre-date the actual tree of devices itself. They were just holders for what the original developer thought was going to be needed, but was never properly implemented due to some job changes (note, this was not myself...) In contrast, the /sys/devices/.../power/state API has never had many users beyond developers trying to test their drivers (without taking the whole system into a low power state, which probably didn't work in any case), and has *always* been problematic. And the change you object to doesn't break anything fundamental, either. Everything still works. It's used on every Ubuntu and Suse system, Odd how the relevant Suse developers didn't mention any issues with those files going away, any of the times problems with them were discussed on the PM list. Also, I have a Suse system that doesn't use those files for anything ... maybe only newer release use it. I would be very interested to see any newer SuSE programs using that interface. Just point them out to me and I'll quickly fix them. And yes, as a SuSE developer (and one of the people in charge of the SuSE kernels), I have no problem with these files just going away. Because, as David keeps repeating, they are broken and wrong. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
On Tue, Dec 19, 2006 at 09:34:17PM -0800, Greg KH wrote: I would be very interested to see any newer SuSE programs using that interface. Just point them out to me and I'll quickly fix them. As far as I can tell, powersaved still uses these.. I'm not quite sure how you can fix it without just removing the functionality from it... And yes, as a SuSE developer (and one of the people in charge of the SuSE kernels), I have no problem with these files just going away. Because, as David keeps repeating, they are broken and wrong. In the common case, it works perfectly well for the management of individual PCI devices. Yes it's wrong, in much the same way as (say) the IDE bus registration/unregistration code. But we keep that around because despite it being even more broken than devices/.../power/state, people are still actually using it and we haven't provided any sort of alternative. Seriously. How many pieces of userspace-visible functionality have recently been removed without there being any sort of alternative? -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Changes to PM layer break userspace
Seriously. How many pieces of userspace-visible functionality have recently been removed without there being any sort of alternative? There IS an alternative, you're using it for networking: You *down the interface*. If there's a NIC that doesn't support that let us (or preferably netdev) know and it'll get fixed quickly I'm sure. -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
