[RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Sysdev have to be suspended and resumed with interrupts disabled and
things usually break in a way that's difficult to debug if one of
sysdev drivers enables interrupts by mistake during suspend or
resume.  Add extra checks that will generate warnings in such cases.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/sys.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Index: linux-2.6/drivers/base/sys.c
===================================================================
--- linux-2.6.orig/drivers/base/sys.c
+++ linux-2.6/drivers/base/sys.c
@@ -343,11 +343,15 @@ static void __sysdev_resume(struct sys_d
 	/* First, call the class-specific one */
 	if (cls->resume)
 		cls->resume(dev);
+	WARN_ONCE(!irqs_disabled(),
+		"Interrupts enabled after %pF\n", cls->resume);
 
 	/* Call auxillary drivers next. */
 	list_for_each_entry(drv, &cls->drivers, entry) {
 		if (drv->resume)
 			drv->resume(dev);
+		WARN_ONCE(!irqs_disabled(),
+			"Interrupts enabled after %pF\n", drv->resume);
 	}
 }
 
@@ -377,6 +381,9 @@ int sysdev_suspend(pm_message_t state)
 	if (ret)
 		return ret;
 
+	WARN_ONCE(!irqs_disabled(),
+		"Interrupts enabled while suspending system devices");
+
 	pr_debug("Suspending System Devices\n");
 
 	list_for_each_entry_reverse(cls, &system_kset->list, kset.kobj.entry) {
@@ -393,6 +400,9 @@ int sysdev_suspend(pm_message_t state)
 					if (ret)
 						goto aux_driver;
 				}
+				WARN_ONCE(!irqs_disabled(),
+					"Interrupts enabled after %pF\n",
+					drv->suspend);
 			}
 
 			/* Now call the generic one */
@@ -400,6 +410,9 @@ int sysdev_suspend(pm_message_t state)
 				ret = cls->suspend(sysdev, state);
 				if (ret)
 					goto cls_driver;
+				WARN_ONCE(!irqs_disabled(),
+					"Interrupts enabled after %pF\n",
+					cls->suspend);
 			}
 		}
 	}
@@ -452,6 +465,9 @@ int sysdev_resume(void)
 {
 	struct sysdev_class *cls;
 
+	WARN_ONCE(!irqs_disabled(),
+		"Interrupts enabled while resuming system devices");
+
 	pr_debug("Resuming System Devices\n");
 
 	list_for_each_entry(cls, &system_kset->list, kset.kobj.entry) {

Re: [RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Frans Pop]

> @@ -377,6 +381,9 @@ int sysdev_suspend(pm_message_t state)
>         if (ret)
>                 return ret;
>  
> +       WARN_ONCE(!irqs_disabled(),
> +               "Interrupts enabled while suspending system devices");
> +

Missing \n ?

> @@ -452,6 +465,9 @@ int sysdev_resume(void)
>  {
>         struct sysdev_class *cls;
>  
> +       WARN_ONCE(!irqs_disabled(),
> +               "Interrupts enabled while resuming system devices");
> +

Again.

Cheers,
FJP

Re: [RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Rafael J. Wysocki]

On Sunday 05 April 2009, Frans Pop wrote:
> > @@ -377,6 +381,9 @@ int sysdev_suspend(pm_message_t state)
> >         if (ret)
> >                 return ret;
> >  
> > +       WARN_ONCE(!irqs_disabled(),
> > +               "Interrupts enabled while suspending system devices");
> > +
> 
> Missing \n ?
> 
> > @@ -452,6 +465,9 @@ int sysdev_resume(void)
> >  {
> >         struct sysdev_class *cls;
> >  
> > +       WARN_ONCE(!irqs_disabled(),
> > +               "Interrupts enabled while resuming system devices");
> > +
> 
> Again.

Yes, thank.

Rafael

Re: [RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Ingo Molnar]

* Rafael J. Wysocki <rjw@sisk.pl> wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Sysdev have to be suspended and resumed with interrupts disabled 
> and things usually break in a way that's difficult to debug if one 
> of sysdev drivers enables interrupts by mistake during suspend or 
> resume.  Add extra checks that will generate warnings in such 
> cases.

no objections - but obviously the real solution would be to finish 
PeterZ's "detect assymetric kernel functions" patch:

That patch (Peter posted an initial version of it already on lkml) 
works the following way:

the ftrace function-graph-tracer plugin is utilized to instrument 
every function call in the kernel. That is extended with the 
following check: every function except __lockfunc and __sched 
functions must be 'balanced' - i.e. they must enter and exit with 
the same IRQ flags, preempt count and lock depth counter.

	Ingo

Re: [RFC][PATCH] PM: Warn if interrupts are enabled during suspend-resume of sysdevs

[Rafael J. Wysocki]

On Sunday 05 April 2009, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Sysdev have to be suspended and resumed with interrupts disabled 
> > and things usually break in a way that's difficult to debug if one 
> > of sysdev drivers enables interrupts by mistake during suspend or 
> > resume.  Add extra checks that will generate warnings in such 
> > cases.
> 
> no objections - but obviously the real solution would be to finish 
> PeterZ's "detect assymetric kernel functions" patch:
> 
> That patch (Peter posted an initial version of it already on lkml) 
> works the following way:
> 
> the ftrace function-graph-tracer plugin is utilized to instrument 
> every function call in the kernel. That is extended with the 
> following check: every function except __lockfunc and __sched 
> functions must be 'balanced' - i.e. they must enter and exit with 
> the same IRQ flags, preempt count and lock depth counter.

Nothing wrong with that, but I'd also like to have checks that don't depend on
ftrace. :-)

Thanks,
Rafael