* Print PCI device in power management warning.
@ 2011-12-23 18:16 Dave Jones
2011-12-23 20:26 ` Konrad Rzeszutek Wilk
2011-12-23 20:44 ` Alan Cox
0 siblings, 2 replies; 5+ messages in thread
From: Dave Jones @ 2011-12-23 18:16 UTC (permalink / raw)
To: Linux Kernel; +Cc: linux-pm
When the WARN_ON in pci_has_legacy_pm_support() triggers, we get
users filing backtraces, but it's not obvious which driver is
triggering the trace. This adds a printk before the BUG.
This still isn't perfect (automated tools like abrt will still miss it)
but we can at least ask the user to look through their dmesg when
we get these traces reported.
Signed-off-by: Dave Jones <davej@redhat.com>
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 12d1e81..b638244 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -604,7 +604,12 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
* supported as well. Drivers are supposed to support either the
* former, or the latter, but not both at the same time.
*/
- WARN_ON(ret && drv->driver.pm);
+ if (ret && drv->driver.pm) {
+ printk(KERN_WARNING "pci: %s has both legacy and new PM support.\n",
+ drv->name);
+ BUG();
+ }
+
return ret;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: Print PCI device in power management warning.
2011-12-23 18:16 Print PCI device in power management warning Dave Jones
@ 2011-12-23 20:26 ` Konrad Rzeszutek Wilk
2011-12-23 20:46 ` Dave Jones
2011-12-23 20:44 ` Alan Cox
1 sibling, 1 reply; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-12-23 20:26 UTC (permalink / raw)
To: Dave Jones, Linux Kernel, linux-pm
On Fri, Dec 23, 2011 at 01:16:26PM -0500, Dave Jones wrote:
> When the WARN_ON in pci_has_legacy_pm_support() triggers, we get
> users filing backtraces, but it's not obvious which driver is
> triggering the trace. This adds a printk before the BUG.
Well, and adds a BUG!
> This still isn't perfect (automated tools like abrt will still miss it)
> but we can at least ask the user to look through their dmesg when
> we get these traces reported.
>
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 12d1e81..b638244 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -604,7 +604,12 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
> * supported as well. Drivers are supposed to support either the
> * former, or the latter, but not both at the same time.
> */
> - WARN_ON(ret && drv->driver.pm);
> + if (ret && drv->driver.pm) {
> + printk(KERN_WARNING "pci: %s has both legacy and new PM support.\n",
> + drv->name);
> + BUG();
Um, so this will hang the machine.
don't you just want:
WARN(ret && drv->driver.pm, "pci: %s Has both legacy and new PM
support!\n", drv_name(drv));
which will do what it previously does and also add the message you
wanted to add?
> + }
> +
>
> return ret;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: Print PCI device in power management warning.
2011-12-23 20:26 ` Konrad Rzeszutek Wilk
@ 2011-12-23 20:46 ` Dave Jones
2011-12-23 22:04 ` Randy Dunlap
0 siblings, 1 reply; 5+ messages in thread
From: Dave Jones @ 2011-12-23 20:46 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Linux Kernel, linux-pm
On Fri, Dec 23, 2011 at 04:26:35PM -0400, Konrad Rzeszutek Wilk wrote:
> don't you just want:
>
> WARN(ret && drv->driver.pm, "pci: %s Has both legacy and new PM
> support!\n", drv_name(drv));
>
> which will do what it previously does and also add the message you
> wanted to add?
close. Not sure what drv_name() is, but I don't seem to have it in my tree.
This works though..
---
When the WARN_ON in pci_has_legacy_pm_support() triggers, we get
users filing backtraces, but it's not obvious which driver is
triggering the trace. Printing the driver name in addition to the trace
should make these easier to debug.
Signed-off-by: Dave Jones <davej@redhat.com>
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 12d1e81..8af9ff2 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -604,7 +604,7 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
* supported as well. Drivers are supposed to support either the
* former, or the latter, but not both at the same time.
*/
- WARN_ON(ret && drv->driver.pm);
+ WARN(ret && drv->driver.pm, "pci: %s Has both legacy and new PM support!\n", drv->name);
return ret;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Print PCI device in power management warning.
2011-12-23 20:46 ` Dave Jones
@ 2011-12-23 22:04 ` Randy Dunlap
0 siblings, 0 replies; 5+ messages in thread
From: Randy Dunlap @ 2011-12-23 22:04 UTC (permalink / raw)
To: Dave Jones, Konrad Rzeszutek Wilk, Linux Kernel, linux-pm
On 12/23/2011 12:46 PM, Dave Jones wrote:
> On Fri, Dec 23, 2011 at 04:26:35PM -0400, Konrad Rzeszutek Wilk wrote:
>
> > don't you just want:
> >
> > WARN(ret && drv->driver.pm, "pci: %s Has both legacy and new PM
> > support!\n", drv_name(drv));
> >
> > which will do what it previously does and also add the message you
> > wanted to add?
>
> close. Not sure what drv_name() is, but I don't seem to have it in my tree.
> This works though..
>
> ---
>
> When the WARN_ON in pci_has_legacy_pm_support() triggers, we get
> users filing backtraces, but it's not obvious which driver is
> triggering the trace. Printing the driver name in addition to the trace
> should make these easier to debug.
>
> Signed-off-by: Dave Jones <davej@redhat.com>
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 12d1e81..8af9ff2 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -604,7 +604,7 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
> * supported as well. Drivers are supposed to support either the
> * former, or the latter, but not both at the same time.
> */
> - WARN_ON(ret && drv->driver.pm);
> + WARN(ret && drv->driver.pm, "pci: %s Has both legacy and new PM support!\n", drv->name);
has
and why not cc: linux-pci?
>
> return ret;
> }
> --
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Print PCI device in power management warning.
2011-12-23 18:16 Print PCI device in power management warning Dave Jones
2011-12-23 20:26 ` Konrad Rzeszutek Wilk
@ 2011-12-23 20:44 ` Alan Cox
1 sibling, 0 replies; 5+ messages in thread
From: Alan Cox @ 2011-12-23 20:44 UTC (permalink / raw)
To: Dave Jones; +Cc: Linux Kernel, linux-pm
On Fri, 23 Dec 2011 13:16:26 -0500
Dave Jones <davej@redhat.com> wrote:
> When the WARN_ON in pci_has_legacy_pm_support() triggers, we get
> users filing backtraces, but it's not obvious which driver is
> triggering the trace. This adds a printk before the BUG.
> This still isn't perfect (automated tools like abrt will still miss it)
> but we can at least ask the user to look through their dmesg when
> we get these traces reported.
>
> Signed-off-by: Dave Jones <davej@redhat.com>
NAK
The old code did a WARN() the new code BUG() which in practice means in
many cases the user will get a crash and hang at boot and not even be
able to catch the trap.
At the very least your changelog should warn people it changes from
WARN_ON noise to system crash and burn.
Alan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-12-23 21:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-23 18:16 Print PCI device in power management warning Dave Jones
2011-12-23 20:26 ` Konrad Rzeszutek Wilk
2011-12-23 20:46 ` Dave Jones
2011-12-23 22:04 ` Randy Dunlap
2011-12-23 20:44 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox