public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Alan Stern <stern@rowland.harvard.edu>,
	"linux-pm" <linux-pm@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>, Len Brown <lenb@kernel.org>,
	Pavel Machek <pavel@ucw.cz>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Arjan van de Ven <arjan@infradead.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Linux PCI <linux-pci@vger.kernel.org>
Subject: [PATCH 10 update] PM: Measure suspend and resume times for individual devices
Date: Mon, 31 Aug 2009 23:32:26 +0200	[thread overview]
Message-ID: <200908312332.26198.rjw@sisk.pl> (raw)
In-Reply-To: <200908311756.54624.rjw@sisk.pl>

On Monday 31 August 2009, Rafael J. Wysocki wrote:
> On Monday 31 August 2009, Ingo Molnar wrote:
...
> > > > Hm, these CPP macros are rather ugly. Why is there a need for 
> > > > the TIMER_DECLARE() wrapper - if a proper inline function is 
> > > > used there's no need for that.
> > > 
> > > I need a variable to be declared so that I can save the "start" 
> > > timestamp in it.  I don't need the variable if DEBUG is unset, 
> > > though.
> > > 
> > > How would you do that without using a macro?  Or #ifdef #endif 
> > > block that would be uglier IMO (which is why I didn't do that)?
> > 
> > Well, why not use an inline function like i suggested? [which does 
> > nothing in the !enabled case] You can keep the local variable always 
> > defined.
> 
> Well, I used the macros _exactly_ because I didn't want to keep the local
> variable always defined.
> 
> Now, if you think that adding several useless bytes to the stack frame is OK,
> perhaps it can be always defined.  However, IMnsHO that would be
> (a) confusing (why define a variable you don't use) and (b) wasteful.
> 
> Perhaps the names of the macros should be directly related to debugging?

Something like in the patch below, maybe?

Best,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM: Measure suspend and resume times for individual devices

If verbose PM debugging is enabled, measure and print the time of
suspending and resuming of individual devices.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/main.c |   51 +++++++++++++++++++++++++++++++++++++++++-----
 kernel/power/swsusp.c     |    2 -
 2 files changed, 47 insertions(+), 6 deletions(-)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -442,11 +442,11 @@ static bool pm_op_started(struct device 
  */
 int pm_time_elapsed(struct timeval *start, struct timeval *stop)
 {
-	s64 elapsed_centisecs64;
+	s64 elapsed_msecs64;
 
-	elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
-	do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
-	return elapsed_centisecs64;
+	elapsed_msecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+	do_div(elapsed_msecs64, NSEC_PER_SEC / 1000);
+	return elapsed_msecs64;
 }
 
 static char *pm_verb(int event)
@@ -476,7 +476,7 @@ static char *pm_verb(int event)
 static void dpm_show_time(struct timeval *start, struct timeval *stop,
 			  pm_message_t state, const char *info)
 {
-	int centisecs = pm_time_elapsed(start, stop);
+	int centisecs = pm_time_elapsed(start, stop) / 10;
 
 	printk(KERN_INFO "PM: %s%s%s of devices complete in %d.%02d seconds\n",
 		info ? info : "", info ? " " : "", pm_verb(state.event),
@@ -497,6 +497,33 @@ static void pm_dev_err(struct device *de
 		kobject_name(&dev->kobj), pm_verb(state.event), info, error);
 }
 
+#ifdef DEBUG
+static void device_show_time(struct timeval *start, struct device *dev,
+			     pm_message_t state, char *info)
+{
+	struct timeval stop;
+	int msecs;
+
+	do_gettimeofday(&stop);
+	msecs = pm_time_elapsed(start, &stop);
+	dev_dbg(dev, "PID %d: %s%s%s complete in %d.%03d seconds\n",
+		task_pid_nr(current), info ? info : "", info ? " " : "",
+		pm_verb(state.event), msecs / 1000, msecs % 1000);
+}
+
+#define DEBUG_TIMEVAL(tv)	struct timeval tv
+#define DEBUG_TIME_START(tv)	do { \
+		do_gettimeofday(&tv); \
+	} while (0)
+#define DEBUG_TIME_SHOW(tv, dev, state, info)	do { \
+		device_show_time(&tv, dev, state, info); \
+	} while (0)
+#else /* !DEBUG */
+#define DEBUG_TIMEVAL(tv)
+#define DEBUG_TIME_START(tv)	do { } while (0)
+#define DEBUG_TIME_SHOW(tv, dev, state, info)	do { } while (0)
+#endif /* !DEBUG */
+
 /*------------------------- Resume routines -------------------------*/
 
 /**
@@ -510,7 +537,9 @@ static void pm_dev_err(struct device *de
 static int __device_resume_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
+	DEBUG_TIMEVAL(start);
 
+	DEBUG_TIME_START(start);
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
@@ -523,6 +552,7 @@ static int __device_resume_noirq(struct 
 	wake_up_all(&dev->power.wait_queue);
 
 	TRACE_RESUME(error);
+	DEBUG_TIME_SHOW(start, dev, state, "EARLY");
 	return error;
 }
 
@@ -639,7 +669,9 @@ EXPORT_SYMBOL_GPL(dpm_resume_noirq);
 static int __device_resume(struct device *dev, pm_message_t state)
 {
 	int error = 0;
+	DEBUG_TIMEVAL(start);
 
+	DEBUG_TIME_START(start);
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
@@ -681,6 +713,7 @@ static int __device_resume(struct device
 	wake_up_all(&dev->power.wait_queue);
 
 	TRACE_RESUME(error);
+	DEBUG_TIME_SHOW(start, dev, state, NULL);
 	return error;
 }
 
@@ -923,6 +956,9 @@ static pm_message_t resume_event(pm_mess
 static int __device_suspend_noirq(struct device *dev, pm_message_t state)
 {
 	int error = 0;
+	DEBUG_TIMEVAL(start);
+
+	DEBUG_TIME_START(start);
 
 	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "LATE ");
@@ -932,6 +968,7 @@ static int __device_suspend_noirq(struct
 	dev->power.op_complete = true;
 	wake_up_all(&dev->power.wait_queue);
 
+	DEBUG_TIME_SHOW(start, dev, state, "LATE");
 	return error;
 }
 
@@ -1063,6 +1100,9 @@ EXPORT_SYMBOL_GPL(dpm_suspend_noirq);
 static int __device_suspend(struct device *dev, pm_message_t state)
 {
 	int error = 0;
+	DEBUG_TIMEVAL(start);
+
+	DEBUG_TIME_START(start);
 
 	down(&dev->sem);
 
@@ -1103,6 +1143,7 @@ static int __device_suspend(struct devic
 	dev->power.op_complete = true;
 	wake_up_all(&dev->power.wait_queue);
 
+	DEBUG_TIME_SHOW(start, dev, state, NULL);
 	return error;
 }
 
Index: linux-2.6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.orig/kernel/power/swsusp.c
+++ linux-2.6/kernel/power/swsusp.c
@@ -169,7 +169,7 @@ int swsusp_swap_in_use(void)
 void swsusp_show_speed(struct timeval *start, struct timeval *stop,
 			unsigned nr_pages, char *msg)
 {
-	int centisecs = pm_time_elapsed(start, stop);
+	int centisecs = pm_time_elapsed(start, stop) / 10;
 	int k;
 	int kps;
 

  reply	other threads:[~2009-08-31 21:31 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-26 20:17 [PATCH 0/6] PM: Asynchronous suspend and resume of devices Rafael J. Wysocki
2009-08-26 20:20 ` [PATCH 1/6] PM: Introduce PM links framework Rafael J. Wysocki
2009-08-28 15:16   ` Alan Stern
2009-08-28 19:11     ` Rafael J. Wysocki
2009-08-26 20:21 ` [PATCH 2/6] PM: Asynchronous resume of devices Rafael J. Wysocki
2009-08-28 15:43   ` Alan Stern
2009-08-28 19:38     ` Rafael J. Wysocki
2009-08-28 19:59       ` Alan Stern
2009-08-28 22:22         ` Rafael J. Wysocki
2009-08-28 23:20           ` Rafael J. Wysocki
2009-08-29  2:06           ` Alan Stern
2009-08-29 12:49             ` Rafael J. Wysocki
2009-08-29 19:17               ` Rafael J. Wysocki
2009-08-30  0:53                 ` Alan Stern
2009-08-30  0:48               ` Alan Stern
2009-08-30 13:15                 ` Rafael J. Wysocki
2009-08-30 21:13                   ` [PATCH 10] PM: Measure suspend and resume times for individual devices (was: Re: [PATCH 2/6] PM: Asynchronous resume of devices) Rafael J. Wysocki
2009-08-31  7:25                     ` Ingo Molnar
2009-08-31 12:53                       ` Rafael J. Wysocki
2009-08-31 13:52                         ` Ingo Molnar
2009-08-31 15:56                           ` Rafael J. Wysocki
2009-08-31 21:32                             ` Rafael J. Wysocki [this message]
2009-09-04  7:51                             ` Ingo Molnar
2009-09-04 14:42                               ` Rafael J. Wysocki
2009-09-04 19:12                                 ` Ingo Molnar
2009-09-04 21:56                                   ` [PATCH 10 update 2x] PM: Measure suspend and resume times for individual devices Rafael J. Wysocki
2009-09-06  4:44                                     ` Ingo Molnar
2009-09-06 12:13                                       ` Rafael J. Wysocki
2009-08-31 14:09                     ` [PATCH 10] PM: Measure suspend and resume times for individual devices (was: Re: [PATCH 2/6] PM: Asynchronous resume of devices) Alan Stern
2009-08-31 16:00                       ` Rafael J. Wysocki
2009-08-30  6:45               ` [PATCH 2/6] PM: Asynchronous resume of devices Pavel Machek
2009-08-30 13:09                 ` Rafael J. Wysocki
2009-08-26 20:21 ` [PATCH 3/6] PM: Asynchronous suspend " Rafael J. Wysocki
2009-08-26 20:22 ` [PATCH 4/6] PM: Allow PCI devices to suspend/resume asynchronously Rafael J. Wysocki
2009-08-26 20:23 ` [PATCH 5/6] PM: Allow ACPI " Rafael J. Wysocki
2009-08-26 20:24 ` [PATCH 6/6] PM: Allow serio input " Rafael J. Wysocki
2009-08-27 20:08   ` Dmitry Torokhov
2009-08-27 20:58     ` Rafael J. Wysocki
2009-08-26 22:25 ` [PATCH 0/6] PM: Asynchronous suspend and resume of devices Rafael J. Wysocki
2009-08-27 19:17   ` Rafael J. Wysocki
2009-08-27 19:19     ` [PATCH 7] PM: Add a switch for disabling/enabling asynchronous suspend/resume Rafael J. Wysocki
2009-08-27 20:07       ` Dmitry Torokhov
2009-08-27 22:22         ` [PATCH 7 updated] " Rafael J. Wysocki
2009-08-28  5:22           ` Dmitry Torokhov
2009-08-28 19:42             ` Rafael J. Wysocki
2009-08-27 19:20     ` [PATCH 8] PM: Allow user space to change the power.async_suspend flag of devices Rafael J. Wysocki
2009-08-27 20:04       ` Dmitry Torokhov
2009-08-27 22:24         ` Rafael J. Wysocki
2009-08-28  7:01           ` Pavel Machek
2009-08-29 19:20             ` [PATCH 8 update] " Rafael J. Wysocki
2009-08-27 20:46 ` [PATCH 0/6] PM: Asynchronous suspend and resume " Alan Stern
2009-08-27 21:18   ` Rafael J. Wysocki
2009-08-29 19:22 ` [PATCH 9] PM: Measure device suspend and resume times (was: Re: [PATCH 0/6] PM: Asynchronous suspend and resume of devices) Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200908312332.26198.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=arjan@infradead.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=mingo@elte.hu \
    --cc=pavel@ucw.cz \
    --cc=rui.zhang@intel.com \
    --cc=stern@rowland.harvard.edu \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox