Re: [PATCH v5] PCI / PM: Tune down retryable runtime suspend error messages

Rafael J. Wysocki
2015-12-02T01:54:45Z

On Monday, November 30, 2015 09:02:55 PM Imre Deak 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. See rpm_suspend() for details of handling these return values.
>
> Note that we use dev_dbg() for the retryable retvals, so after this
> change you'll need either CONFIG_DYNAMIC_DEBUG or CONFIG_PCI_DEBUG
> for them to show up in the log.
>
> 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.
>
> 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>

Hi Bjorn,

Are you going to handle this one or should I take care of it?

Rafael

> ---
>
> 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)
> v4:
> - don't refer to log levels as flags in code comment (Rafael)
> - use pr_debug(), pr_err() instead of the corresponding printk() (Rafael)
> v5:
> - clarify commit message (Bjorn)
> - use dev_dbg, dev_err instead of pr_debug, pr_err (Bjorn)
> - use %pf in printk format instead of %pF (Bjorn)
> - make the debug/error messages more meaningful (Bjorn)
> ---
> drivers/pci/pci-driver.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 4446fcb..d7ffd66 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1146,9 +1146,21 @@ 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 log the event only with debug
> + * log level.
> + */
> + if (error == -EBUSY || error == -EAGAIN)
> + dev_dbg(dev, "can't suspend now (%pf returned %d)\n",
> + pm->runtime_suspend, error);
> + else
> + dev_err(dev, "can't suspend (%pf returned %d)\n",
> + pm->runtime_suspend, error);
> +
> return error;
> + }
> if (!pci_dev->d3cold_allowed)
> pci_dev->no_d3cold = true;
>
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.