From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v4] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval Date: Mon, 30 Nov 2015 03:07:26 +0100 Message-ID: <5110079.njQQ6VzEqu@vostro.rjw.lan> References: <1448648254-12639-1-git-send-email-imre.deak@intel.com> <1448699664-4178-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from v094114.home.net.pl ([79.96.170.134]:45565 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751340AbbK3Bhk (ORCPT ); Sun, 29 Nov 2015 20:37:40 -0500 In-Reply-To: <1448699664-4178-1-git-send-email-imre.deak@intel.com> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Imre Deak , Bjorn Helgaas Cc: Jani Nikula , Daniel Vetter , intel-gfx@lists.freedesktop.org, Linux PCI , Linux PM On Saturday, November 28, 2015 10:34:24 AM 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. > > 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) > v4: > - don't refer to log levels as flags in code comment (Rafael) > - use pr_debug(), pr_err() instead of the corresponding printk() (Rafael) > > Reported-by: Daniel Vetter > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92992 > CC: Bjorn Helgaas > CC: Rafael J. Wysocki > Signed-off-by: Imre Deak This is fine by me. Bjorn, any objections against applying it? It reduces log noise quite a bit for i915. > --- > 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..32a9947 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) > + pr_debug("%s(): %pF returns %d\n", __func__, > + pm->runtime_suspend, error); > + else > + pr_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