[LUCID 1/2] UBUNTU: SAUCE: PM report driver and device suspend/resume times.
Amit Kucheria
amit.kucheria at canonical.com
Fri Feb 12 19:17:22 UTC 2010
On 10 Feb 12, Manoy Iyer wrote:
>
> I had sent Rafael's patch earlier to the list, I will follow what amit,
> cherry pick Rafael's, and merge my changes in and resend it.
Just to be clear, don't merge your changes into that patch. Make it separate.
Cheers,
Amit
> Cheers
> --- manjo
>
> On Fri, 12 Feb 2010, Andy Whitcroft wrote:
>
> > On Thu, Feb 11, 2010 at 01:55:35PM -0600, Manoj Iyer wrote:
> >>
> >> This patch is an extension to a resume report patch submitted to upstream
> >> by Rafael J. Wysocki. cnd tested this kernel and reported that it prints
> >> information correctly. The messages will be printed if suspend/resume of
> >> driver/device took more than 100ms. This default can be changed in CONFIG.
> >
> > If we require another patch you should send it in here so we have the
> > full history. If that patch is full contained in this one then we should
> > likely pull that one out in its original form for both attribution and
> > to avoid collisions when we rebase up to that version in the future.
> >
> >> Feb 11 13:23:05 cndougla kernel: [ 59.848676] PM: suspend of drv:sd
> >> dev:0:0:0:0 complete after 978.535 msecs
> >> Feb 11 13:23:05 cndougla kernel: [ 60.110097] PM: suspend of drv:HDA
> >> Intel dev:0000:00:08.0 complete after 149.996 msecs
> >> Feb 11 13:23:05 cndougla kernel: [ 60.110433] PM: suspend devices took
> >> 1.320 seconds
> >>
> >> Feb 11 13:23:05 cndougla kernel: [ 61.313617] PM: resume of drv:battery
> >> dev:PNP0C0A:00 complete after 141.572 msecs
> >> Feb 11 13:23:05 cndougla kernel: [ 66.300578] PM: resume of drv:nvidia
> >> dev:0000:02:00.0 complete after 4849.583 msecs
> >
> > Yeeks how slow is Nvidia. This is exactly the sort of information we
> > are looking to get out of the debugging exercise.
> >
> >> Feb 11 13:23:05 cndougla kernel: [ 66.562640] PM: resume of drv:usb
> >> dev:usb3 complete after 109.993 msecs
> >> Feb 11 13:23:05 cndougla kernel: [ 66.672636] PM: resume of drv:usb
> >> dev:usb4 complete after 109.980 msecs
> >> Feb 11 13:23:05 cndougla kernel: [ 67.076900] PM: resume devices took
> >> 5.900 seconds
> >>
> >>
> >> The following changes since commit
> >> a913ef9ba3b23aee8db094d5ab5804777c697ef1:
> >> Andy Whitcroft (1):
> >> UBUNTU: Ubuntu-2.6.32-13.18
> >>
> >> are available in the git repository at:
> >>
> >>
> >> ssh://zinc.canonical.com/srv/kernel.ubuntu.com/git/manjo/ubuntu-lucid.git
> >> srtime
> >>
> >> Manoj Iyer (2):
> >> UBUNTU: SAUCE: PM report driver and device suspend/resume times.
> >> UBUNTU: [Config] Added CONFIG_SR_REPORT_TIME_LIMIT
> >>
> >> debian.master/config/config.common.ubuntu | 1 +
> >> drivers/base/power/main.c | 35
> >> +++++++++++++++++++++++++++++
> >> 2 files changed, 36 insertions(+), 0 deletions(-)
> >>
> >>
> >> From b5c3874a49410d148d662b42bf2d305e83ddfc3e Mon Sep 17 00:00:00 2001
> >> From: Manoj Iyer <manoj.iyer at canonical.com>
> >> Date: Thu, 11 Feb 2010 13:35:42 -0600
> >> Subject: [PATCH 1/2] UBUNTU: SAUCE: PM report driver and device suspend/resume times.
> >>
> >> This patch prints suspend/resume information for each driver/device to
> >> dmesg. I don't think it adds much value to upstream.
> >>
> >> Signed-off-by: Manoj Iyer <manoj.iyer at canonical.com>
> >> ---
> >> drivers/base/power/main.c | 35 +++++++++++++++++++++++++++++++++++
> >> 1 files changed, 35 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> >> index 8aa2443..578b43d 100644
> >> --- a/drivers/base/power/main.c
> >> +++ b/drivers/base/power/main.c
> >> @@ -29,6 +29,13 @@
> >> #include "../base.h"
> >> #include "power.h"
> >>
> >> +/* Set default to 100 msecs */
> >> +#ifdef CONFIG_SR_REPORT_TIME_LIMIT
> >> +#define SR_REPORT_TIME_LIMIT CONFIG_SR_REPORT_TIME_LIMIT
> >> +#else
> >> +#define SR_REPORT_TIME_LIMIT 100
> >> +#endif
> >> +
> >
> > If this is a configuration option you should be handling the default
> > value in the definition of the confuration option and so shouldn't need
> > this here. You should be able to rely on CONFIG_FOO existing and having
> > a proper value and just use it in place below.
> >
> >> /*
> >> * The entries in the dpm_list list are in a depth first order, simply
> >> * because children are guaranteed to be discovered after parents, and
> >> @@ -324,6 +331,26 @@ static void pm_dev_err(struct device *dev, pm_message_t state, char *info,
> >> kobject_name(&dev->kobj), pm_verb(state.event), info, error);
> >> }
> >>
> >> +
> >> +static void device_show_time(struct device *dev, ktime_t starttime, pm_message_t state, char *info)
> >> +{
> >> + ktime_t calltime;
> >> + s64 usecs64;
> >> + int usecs;
> >> +
> >> + calltime = ktime_get();
> >> + usecs64 = ktime_to_ns(ktime_sub(calltime, starttime));
> >> + do_div(usecs64, NSEC_PER_USEC);
> >> + usecs = usecs64;
> >> + if (usecs == 0)
> >> + usecs = 1;
> >> + if ((usecs / USEC_PER_MSEC) > SR_REPORT_TIME_LIMIT)
> >> + pr_info("PM: %s%s%s of drv:%s dev:%s complete after %ld.%03ld msecs\n", info ?: "", info ? " " : "", pm_verb(state.event),
> >> + dev_driver_string(dev), dev_name(dev), usecs / USEC_PER_MSEC,
> >> + usecs % USEC_PER_MSEC);
> >> +}
> >> +
> >> +
> >> /*------------------------- Resume routines -------------------------*/
> >>
> >> /**
> >> @@ -337,6 +364,7 @@ static void pm_dev_err(struct device *dev, pm_message_t state, char *info,
> >> static int device_resume_noirq(struct device *dev, pm_message_t state)
> >> {
> >> int error = 0;
> >> + ktime_t starttime = ktime_get();
> >>
> >> TRACE_DEVICE(dev);
> >> TRACE_RESUME(0);
> >> @@ -347,6 +375,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state)
> >> if (dev->bus->pm) {
> >> pm_dev_dbg(dev, state, "EARLY ");
> >> error = pm_noirq_op(dev, dev->bus->pm, state);
> >> + device_show_time(dev, starttime, state, "early");
> >> }
> >> End:
> >> TRACE_RESUME(error);
> >> @@ -388,6 +417,7 @@ EXPORT_SYMBOL_GPL(dpm_resume_noirq);
> >> static int device_resume(struct device *dev, pm_message_t state)
> >> {
> >> int error = 0;
> >> + ktime_t starttime = ktime_get();
> >>
> >> TRACE_DEVICE(dev);
> >> TRACE_RESUME(0);
> >> @@ -424,6 +454,7 @@ static int device_resume(struct device *dev, pm_message_t state)
> >> error = dev->class->resume(dev);
> >> }
> >> }
> >> + device_show_time(dev, starttime, state, NULL);
> >> End:
> >> up(&dev->sem);
> >>
> >> @@ -583,6 +614,7 @@ static pm_message_t resume_event(pm_message_t sleep_state)
> >> static int device_suspend_noirq(struct device *dev, pm_message_t state)
> >> {
> >> int error = 0;
> >> + ktime_t starttime = ktime_get();
> >>
> >> if (!dev->bus)
> >> return 0;
> >> @@ -590,6 +622,7 @@ static int device_suspend_noirq(struct device *dev, pm_message_t state)
> >> if (dev->bus->pm) {
> >> pm_dev_dbg(dev, state, "LATE ");
> >> error = pm_noirq_op(dev, dev->bus->pm, state);
> >> + device_show_time(dev, starttime, state, "late");
> >> }
> >> return error;
> >> }
> >> @@ -631,6 +664,7 @@ EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
> >> static int device_suspend(struct device *dev, pm_message_t state)
> >> {
> >> int error = 0;
> >> + ktime_t starttime = ktime_get();
> >>
> >> down(&dev->sem);
> >>
> >> @@ -666,6 +700,7 @@ static int device_suspend(struct device *dev, pm_message_t state)
> >> suspend_report_result(dev->bus->suspend, error);
> >> }
> >> }
> >> + device_show_time(dev, starttime, state, NULL);
> >> End:
> >> up(&dev->sem);
> >>
> >
> > You introduce what appears to be a new CONFIG_ option but do not
> > introduce a Kconfig fragment to create it? Where did that go?
> >
> > -apw
> >
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
--
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || amit.kucheria at canonical.com
----------------------------------------------------------------------
More information about the kernel-team
mailing list