diff mbox

[v3] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval

Message ID 1448648254-12639-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Imre Deak Nov. 27, 2015, 6:17 p.m. UTC
The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
suspend hooks as errors, but they still show up as errors in dmesg. Tune
them down.

One problem caused by this was noticed by Daniel: the i915 driver
returns EAGAIN to signal a temporary failure to suspend and as a request
towards the RPM core for scheduling a suspend again. This is a normal
event, but the resulting error message flags a breakage during the
driver's automated testing which parses dmesg and picks up the error.

v2:
- fix compile breake when CONFIG_PM_SLEEP=n (0-day builder)
v3:
- instead of modifying the suspend_report_result() helper to disinguish
  between the runtime and system suspend case, inline the error
  printing, it's not used anywhere else (Rafael)

Reported-by: Daniel Vetter <daniel.vetter@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92992
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/pci/pci-driver.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki Nov. 27, 2015, 10:23 p.m. UTC | #1
On Fri, Nov 27, 2015 at 7:17 PM, Imre Deak <imre.deak@intel.com> wrote:
> The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
> suspend hooks as errors, but they still show up as errors in dmesg. Tune
> them down.
>
> One problem caused by this was noticed by Daniel: the i915 driver
> returns EAGAIN to signal a temporary failure to suspend and as a request
> towards the RPM core for scheduling a suspend again. This is a normal
> event, but the resulting error message flags a breakage during the
> driver's automated testing which parses dmesg and picks up the error.
>
> v2:
> - fix compile breake when CONFIG_PM_SLEEP=n (0-day builder)
> v3:
> - instead of modifying the suspend_report_result() helper to disinguish
>   between the runtime and system suspend case, inline the error
>   printing, it's not used anywhere else (Rafael)
>
> Reported-by: Daniel Vetter <daniel.vetter@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92992
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/pci/pci-driver.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 4446fcb..67eb4ac 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1146,9 +1146,20 @@ static int pci_pm_runtime_suspend(struct device *dev)
>         pci_dev->state_saved = false;
>         pci_dev->no_d3cold = false;
>         error = pm->runtime_suspend(dev);
> -       suspend_report_result(pm->runtime_suspend, error);
> -       if (error)
> +       if (error) {
> +               /*
> +                * -EBUSY and -EAGAIN is used to request the runtime PM core
> +                * to schedule a new suspend, so don't flag it as an error.
> +                */

KERN_DEBUG and KERN_ERR are log levels, not flags.

Besides, maybe we can use pr_err() and pr_debug() here?

> +               if (error == -EBUSY || error == -EAGAIN)
> +                       printk(KERN_DEBUG "%s(): %pF returns %d\n", __func__,
> +                              pm->runtime_suspend, error);
> +               else
> +                       printk(KERN_ERR "%s(): %pF returns %d\n", __func__,
> +                              pm->runtime_suspend, error);
> +
>                 return error;
> +       }
>         if (!pci_dev->d3cold_allowed)
>                 pci_dev->no_d3cold = true;
>
> --

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 4446fcb..67eb4ac 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1146,9 +1146,20 @@  static int pci_pm_runtime_suspend(struct device *dev)
 	pci_dev->state_saved = false;
 	pci_dev->no_d3cold = false;
 	error = pm->runtime_suspend(dev);
-	suspend_report_result(pm->runtime_suspend, error);
-	if (error)
+	if (error) {
+		/*
+		 * -EBUSY and -EAGAIN is used to request the runtime PM core
+		 * to schedule a new suspend, so don't flag it as an error.
+		 */
+		if (error == -EBUSY || error == -EAGAIN)
+			printk(KERN_DEBUG "%s(): %pF returns %d\n", __func__,
+			       pm->runtime_suspend, error);
+		else
+			printk(KERN_ERR "%s(): %pF returns %d\n", __func__,
+			       pm->runtime_suspend, error);
+
 		return error;
+	}
 	if (!pci_dev->d3cold_allowed)
 		pci_dev->no_d3cold = true;