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

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Dec 02, 2015 at 02:54:45AM +0100, Rafael J. Wysocki wrote:
> 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?

Why don't you take it, since you're the PM guru :)

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Thanks for all your work, Imre!

> > ---
> > 
> > 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.
> --
> 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