[PATCH 0/3] perf, tools: new power trace API

[PATCH 0/3] perf, tools: new power trace API

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Provides:
. calls to machine_suspend trace point,
. OMAP support,
. API Documentation

Applies on top of Thomas's 8 latest power trace API patches, cf.
http://marc.info/?l=linux-kernel&m=129130827309354&w=2

Jean Pihet (3):
  perf: add calls to suspend trace point
  perf: add OMAP support for the new power events
  tools, perf: Documentation for the power events API

 Documentation/trace/events-power.txt |   90 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm34xx.c         |    7 +++
 arch/arm/mach-omap2/powerdomain.c    |    3 +
 arch/arm/plat-omap/clock.c           |   13 ++++-
 kernel/power/suspend.c               |    3 +
 5 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/trace/events-power.txt

-- 
1.7.2.3

[PATCH 1/3] perf: add calls to suspend trace point

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
 kernel/power/suspend.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <trace/events/power.h>
 
 #include "power.h"
 
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
 	error = sysdev_suspend(PMSG_SUSPEND);
 	if (!error) {
 		if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+			trace_machine_suspend(state);
 			error = suspend_ops->enter(state);
+			trace_machine_suspend(PWR_EVENT_EXIT);
 			events_check_enabled = false;
 		}
 		sysdev_resume();
-- 
1.7.2.3

[PATCH 2/3] perf: add OMAP support for the new power events

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

The patch adds the new power management trace points for
the OMAP architecture.

The trace points are for:
- default idle handler. Since the cpuidle framework is
  instrumented in the generic way there is no need to
  add trace points in the OMAP specific cpuidle handler;
- cpufreq (DVFS),
- clocks changes (enable, disable, set_rate),
- change of power domains next power states.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c      |    7 +++++++
 arch/arm/mach-omap2/powerdomain.c |    3 +++
 arch/arm/plat-omap/clock.c        |   13 ++++++++++---
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0ec8a04..0ee0b0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -29,6 +29,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/console.h>
+#include <trace/events/power.h>
 
 #include <plat/sram.h>
 #include <plat/clockdomain.h>
@@ -506,8 +507,14 @@ static void omap3_pm_idle(void)
 	if (omap_irq_pending() || need_resched())
 		goto out;
 
+	trace_power_start(POWER_CSTATE, 1, smp_processor_id());
+	trace_cpu_idle(1, smp_processor_id());
+
 	omap_sram_idle();
 
+	trace_power_end(smp_processor_id());
+	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
 out:
 	local_fiq_enable();
 	local_irq_enable();
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 6527ec3..73cbe9a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -23,6 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <trace/events/power.h>
 
 #include <asm/atomic.h>
 
@@ -440,6 +441,8 @@ int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
+	trace_power_domain_target(pwrdm->name, pwrst, smp_processor_id());
+
 	prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
 			     (pwrst << OMAP_POWERSTATE_SHIFT),
 			     pwrdm->prcm_offs, pwrstctrl_reg_offs);
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index fc62fb5..7cbb09b 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -21,6 +21,7 @@
 #include <linux/cpufreq.h>
 #include <linux/debugfs.h>
 #include <linux/io.h>
+#include <trace/events/power.h>
 
 #include <plat/clock.h>
 
@@ -43,8 +44,10 @@ int clk_enable(struct clk *clk)
 		return -EINVAL;
 
 	spin_lock_irqsave(&clockfw_lock, flags);
-	if (arch_clock->clk_enable)
+	if (arch_clock->clk_enable) {
+		trace_clock_enable(clk->name, 1, smp_processor_id());
 		ret = arch_clock->clk_enable(clk);
+	}
 	spin_unlock_irqrestore(&clockfw_lock, flags);
 
 	return ret;
@@ -66,8 +69,10 @@ void clk_disable(struct clk *clk)
 		goto out;
 	}
 
-	if (arch_clock->clk_disable)
+	if (arch_clock->clk_disable) {
+		trace_clock_disable(clk->name, 0, smp_processor_id());
 		arch_clock->clk_disable(clk);
+	}
 
 out:
 	spin_unlock_irqrestore(&clockfw_lock, flags);
@@ -120,8 +125,10 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
 		return ret;
 
 	spin_lock_irqsave(&clockfw_lock, flags);
-	if (arch_clock->clk_set_rate)
+	if (arch_clock->clk_set_rate) {
+		trace_clock_set_rate(clk->name, rate, smp_processor_id());
 		ret = arch_clock->clk_set_rate(clk, rate);
+	}
 	if (ret == 0) {
 		if (clk->recalc)
 			clk->rate = clk->recalc(clk);
-- 
1.7.2.3

[PATCH 3/3] tools, perf: Documentation for the power events API

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Provides documentation for the following:
- the new power trace API,
- the old (legacy) power trace API,
- the DEPRECATED Kconfig option usage..

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 Documentation/trace/events-power.txt |   90 ++++++++++++++++++++++++++++++++++
 1 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/trace/events-power.txt

diff --git a/Documentation/trace/events-power.txt b/Documentation/trace/events-power.txt
new file mode 100644
index 0000000..8a50653
--- /dev/null
+++ b/Documentation/trace/events-power.txt
@@ -0,0 +1,90 @@
+
+			Subsystem Trace Points: power
+
+The power tracing system captures events related to power transitions
+within the kernel. Broadly speaking there are three major subheadings:
+
+  o Power state switch which reports events related to suspend (S-states),
+     cpuidle (C-states) and cpufreq (P-states)
+  o System clock related changes
+  o Power domains related changes and transitions
+
+This document describes what each of the tracepoints is and why they
+might be useful.
+
+Cf. include/trace/events/power.h for the events definitions.
+
+1. Power state switch events
+============================
+
+1.1 New trace API
+-----------------
+
+A 'cpu' event class gathers the CPU-related events: cpuidle and
+cpufreq.
+
+cpu_idle		"state=%lu cpu_id=%lu"
+cpu_frequency		"state=%lu cpu_id=%lu"
+
+A suspend event is used to indicate the system going in and out of the
+suspend mode:
+
+machine_suspend		"state=%lu"
+
+
+Note: the value of '-1' or '4294967295' for state means an exit from the current state,
+i.e. trace_cpu_idle(4, smp_processor_id()) means that the system
+enters the idle state 4, while trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id())
+means that the system exits the previous idle state.
+
+The event which has 'state=4294967295' in the trace is very important to the user
+space tools which are using it to detect the end of the current state, and so to
+correctly draw the states diagrams and to calculate accurate statistics etc.
+
+1.2 DEPRECATED trace API
+------------------------
+
+A new Kconfig option CONFIG_EVENT_POWER_TRACING_DEPRECATED with the default value of
+'y' has been created. This allows the legacy trace power API to be used conjointly
+with the new trace API.
+The Kconfig option, the old trace API (in include/trace/events/power.h) and the
+old trace points will disappear in a future release (namely 2.6.41).
+
+power_start		"type=%lu state=%lu cpu_id=%lu"
+power_frequency		"type=%lu state=%lu cpu_id=%lu"
+power_end		"cpu_id=%lu"
+
+The 'type' parameter takes one of those macros:
+ . POWER_NONE	= 0,
+ . POWER_CSTATE	= 1,	/* C-State */
+ . POWER_PSTATE	= 2,	/* Fequency change or DVFS */
+
+The 'state' parameter is set depending on the type:
+ . Target C-state for type=POWER_CSTATE,
+ . Target frequency for type=POWER_PSTATE,
+
+power_end is used to indicate the exit of a state, corresponding to the latest
+power_start event.
+
+2. Clocks events
+================
+The clock events are used for clock enable/disable and for
+clock rate change.
+
+clock_enable		"%s state=%lu cpu_id=%lu"
+clock_disable		"%s state=%lu cpu_id=%lu"
+clock_set_rate		"%s state=%lu cpu_id=%lu"
+
+The first parameter gives the clock name (e.g. "gpio1_iclk").
+The second parameter is '1' for enable, '0' for disable, the target
+clock rate for set_rate.
+
+3. Power domains events
+=======================
+The power domain events are used for power domains transitions
+
+power_domain_target	"%s state=%lu cpu_id=%lu"
+
+The first parameter gives the power domain name (e.g. "mpu_pwrdm").
+The second parameter is the power domain target state.
+
-- 
1.7.2.3

[PATCH 0/3] perf, tools: new power trace API

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Provides:
. calls to machine_suspend trace point,
. OMAP support,
. API Documentation

Applies on top of Thomas's 8 latest power trace API patches, cf.
http://marc.info/?l=linux-kernel&m=129130827309354&w=2

Jean Pihet (3):
  perf: add calls to suspend trace point
  perf: add OMAP support for the new power events
  tools, perf: Documentation for the power events API

 Documentation/trace/events-power.txt |   90 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm34xx.c         |    7 +++
 arch/arm/mach-omap2/powerdomain.c    |    3 +
 arch/arm/plat-omap/clock.c           |   13 ++++-
 kernel/power/suspend.c               |    3 +
 5 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/trace/events-power.txt

-- 
1.7.2.3

[PATCH 1/3] perf: add calls to suspend trace point

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
 kernel/power/suspend.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <trace/events/power.h>
 
 #include "power.h"
 
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
 	error = sysdev_suspend(PMSG_SUSPEND);
 	if (!error) {
 		if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+			trace_machine_suspend(state);
 			error = suspend_ops->enter(state);
+			trace_machine_suspend(PWR_EVENT_EXIT);
 			events_check_enabled = false;
 		}
 		sysdev_resume();
-- 
1.7.2.3

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Ingo Molnar]

* jean.pihet@newoldbits.com <jean.pihet@newoldbits.com> wrote:

> From: Jean Pihet <j-pihet@ti.com>
> 
> Uses the machine_suspend trace point, called from the
> generic kernel suspend_enter function.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
>  kernel/power/suspend.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Please use the scripts/get_maintainer.pl to construct a proper Cc: list and to 
gather the necessary Acked-by:

  scripts/get_maintainer.pl -f kernel/power/suspend.c

Thanks,

	Ingo

[PATCH 1/3] perf: add calls to suspend trace point

[jean.pihet]

From: Jean Pihet <j-pihet@ti.com>

Uses the machine_suspend trace point, called from the
generic kernel suspend_enter function.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
CC: Thomas Renninger <trenn@suse.de>
---
 kernel/power/suspend.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index ecf7705..0650596 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -22,6 +22,7 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <trace/events/power.h>
 
 #include "power.h"
 
@@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
 	error = sysdev_suspend(PMSG_SUSPEND);
 	if (!error) {
 		if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
+			trace_machine_suspend(state);
 			error = suspend_ops->enter(state);
+			trace_machine_suspend(PWR_EVENT_EXIT);
 			events_check_enabled = false;
 		}
 		sysdev_resume();
-- 
1.7.2.3

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Pavel Machek]

Hi!

> Uses the machine_suspend trace point, called from the
> generic kernel suspend_enter function.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> CC: Thomas Renninger <trenn@suse.de>
> ---
>  kernel/power/suspend.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index ecf7705..0650596 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -22,6 +22,7 @@
>  #include <linux/mm.h>
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
> +#include <trace/events/power.h>
>  
>  #include "power.h"
>  
> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>  	error = sysdev_suspend(PMSG_SUSPEND);
>  	if (!error) {
>  		if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> +			trace_machine_suspend(state);
>  			error = suspend_ops->enter(state);
> +			trace_machine_suspend(PWR_EVENT_EXIT);
>  			events_check_enabled = false;
>  		}
>  		sysdev_resume();

Ok... why this place? I mean, perhaps suspend time should include
device suspend?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Jean Pihet]

Hi,

On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Uses the machine_suspend trace point, called from the
>> generic kernel suspend_enter function.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> CC: Thomas Renninger <trenn@suse.de>
>> ---
>>  kernel/power/suspend.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> index ecf7705..0650596 100644
>> --- a/kernel/power/suspend.c
>> +++ b/kernel/power/suspend.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/mm.h>
>>  #include <linux/slab.h>
>>  #include <linux/suspend.h>
>> +#include <trace/events/power.h>
>>
>>  #include "power.h"
>>
>> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>>       error = sysdev_suspend(PMSG_SUSPEND);
>>       if (!error) {
>>               if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> +                     trace_machine_suspend(state);
>>                       error = suspend_ops->enter(state);
>> +                     trace_machine_suspend(PWR_EVENT_EXIT);
>>                       events_check_enabled = false;
>>               }
>>               sysdev_resume();
>
> Ok... why this place?
This trace has been placed here because it traces the machine low
level mode enter.

> I mean, perhaps suspend time should include
> device suspend?
That makes sense. We have a few options here:
1) keep the traces as proposed to trace the low level machine code only,
2) move the traces to the entry and exit of suspend_enter so that it
includes the prepare and late_prepare (+ the associated wake-up)
callbacks as well,
3) move the traces to suspend_devices_and_enter so that it includes 2)
and the handling of the console and the devices,
4) move the traces to enter_state do that it includes 3), the call to
sys_sync and the user space freeze.

Note that the the SNAPSHOT_2RAM ioctl code also calls
suspend_devices_and_enter, so if only 4) is used no trace will be
generated in that case.

I am in favor of 3) of 4).
What do you think?

>                                                                        Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>

Thanks,
Jean

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Rafael J. Wysocki]

On Tuesday, January 04, 2011, Jean Pihet wrote:
> Hi,
> 
> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> Uses the machine_suspend trace point, called from the
> >> generic kernel suspend_enter function.
> >>
> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> CC: Thomas Renninger <trenn@suse.de>
> >> ---
> >>  kernel/power/suspend.c |    3 +++
> >>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> index ecf7705..0650596 100644
> >> --- a/kernel/power/suspend.c
> >> +++ b/kernel/power/suspend.c
> >> @@ -22,6 +22,7 @@
> >>  #include <linux/mm.h>
> >>  #include <linux/slab.h>
> >>  #include <linux/suspend.h>
> >> +#include <trace/events/power.h>
> >>
> >>  #include "power.h"
> >>
> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >>       error = sysdev_suspend(PMSG_SUSPEND);
> >>       if (!error) {
> >>               if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> +                     trace_machine_suspend(state);
> >>                       error = suspend_ops->enter(state);
> >> +                     trace_machine_suspend(PWR_EVENT_EXIT);
> >>                       events_check_enabled = false;
> >>               }
> >>               sysdev_resume();
> >
> > Ok... why this place?
> This trace has been placed here because it traces the machine low
> level mode enter.
> 
> > I mean, perhaps suspend time should include
> > device suspend?
> That makes sense. We have a few options here:
> 1) keep the traces as proposed to trace the low level machine code only,
> 2) move the traces to the entry and exit of suspend_enter so that it
> includes the prepare and late_prepare (+ the associated wake-up)
> callbacks as well,
> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> and the handling of the console and the devices,
> 4) move the traces to enter_state do that it includes 3), the call to
> sys_sync and the user space freeze.
> 
> Note that the the SNAPSHOT_2RAM ioctl code also calls
> suspend_devices_and_enter, so if only 4) is used no trace will be
> generated in that case.
> 
> I am in favor of 3) of 4).
> What do you think?

Why don't we keep the tracepoints as proposed _and_ add two additional
tracepoints around device suspend-resume?

Rafael

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Jean Pihet]

On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, January 04, 2011, Jean Pihet wrote:
>> Hi,
>>
>> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >> Uses the machine_suspend trace point, called from the
>> >> generic kernel suspend_enter function.
>> >>
>> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> >> CC: Thomas Renninger <trenn@suse.de>
>> >> ---
>> >>  kernel/power/suspend.c |    3 +++
>> >>  1 files changed, 3 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
>> >> index ecf7705..0650596 100644
>> >> --- a/kernel/power/suspend.c
>> >> +++ b/kernel/power/suspend.c
>> >> @@ -22,6 +22,7 @@
>> >>  #include <linux/mm.h>
>> >>  #include <linux/slab.h>
>> >>  #include <linux/suspend.h>
>> >> +#include <trace/events/power.h>
>> >>
>> >>  #include "power.h"
>> >>
>> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
>> >>       error = sysdev_suspend(PMSG_SUSPEND);
>> >>       if (!error) {
>> >>               if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
>> >> +                     trace_machine_suspend(state);
>> >>                       error = suspend_ops->enter(state);
>> >> +                     trace_machine_suspend(PWR_EVENT_EXIT);
>> >>                       events_check_enabled = false;
>> >>               }
>> >>               sysdev_resume();
>> >
>> > Ok... why this place?
>> This trace has been placed here because it traces the machine low
>> level mode enter.
>>
>> > I mean, perhaps suspend time should include
>> > device suspend?
>> That makes sense. We have a few options here:
>> 1) keep the traces as proposed to trace the low level machine code only,
>> 2) move the traces to the entry and exit of suspend_enter so that it
>> includes the prepare and late_prepare (+ the associated wake-up)
>> callbacks as well,
>> 3) move the traces to suspend_devices_and_enter so that it includes 2)
>> and the handling of the console and the devices,
>> 4) move the traces to enter_state do that it includes 3), the call to
>> sys_sync and the user space freeze.
>>
>> Note that the the SNAPSHOT_2RAM ioctl code also calls
>> suspend_devices_and_enter, so if only 4) is used no trace will be
>> generated in that case.
>>
>> I am in favor of 3) of 4).
>> What do you think?
>
> Why don't we keep the tracepoints as proposed _and_ add two additional
> tracepoints around device suspend-resume?
I like the idea but that requires to extend the current API with
additional suspend tracepoints or device state change tracepoints.
That can be done once the current API is firmly in place.
Today the only trace API for suspend is machine_suspend(unsigned int
state), so I think the best option is 3) here above.

Unless there is an objection I am pushing 3) asap.

Thanks!
Jean

>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Rafael J. Wysocki]

On Wednesday, January 05, 2011, Jean Pihet wrote:
> On Wed, Jan 5, 2011 at 12:01 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday, January 04, 2011, Jean Pihet wrote:
> >> Hi,
> >>
> >> On Tue, Jan 4, 2011 at 12:29 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> > Hi!
> >> >
> >> >> Uses the machine_suspend trace point, called from the
> >> >> generic kernel suspend_enter function.
> >> >>
> >> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> >> CC: Thomas Renninger <trenn@suse.de>
> >> >> ---
> >> >>  kernel/power/suspend.c |    3 +++
> >> >>  1 files changed, 3 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> >> >> index ecf7705..0650596 100644
> >> >> --- a/kernel/power/suspend.c
> >> >> +++ b/kernel/power/suspend.c
> >> >> @@ -22,6 +22,7 @@
> >> >>  #include <linux/mm.h>
> >> >>  #include <linux/slab.h>
> >> >>  #include <linux/suspend.h>
> >> >> +#include <trace/events/power.h>
> >> >>
> >> >>  #include "power.h"
> >> >>
> >> >> @@ -164,7 +165,9 @@ static int suspend_enter(suspend_state_t state)
> >> >>       error = sysdev_suspend(PMSG_SUSPEND);
> >> >>       if (!error) {
> >> >>               if (!suspend_test(TEST_CORE) && pm_check_wakeup_events()) {
> >> >> +                     trace_machine_suspend(state);
> >> >>                       error = suspend_ops->enter(state);
> >> >> +                     trace_machine_suspend(PWR_EVENT_EXIT);
> >> >>                       events_check_enabled = false;
> >> >>               }
> >> >>               sysdev_resume();
> >> >
> >> > Ok... why this place?
> >> This trace has been placed here because it traces the machine low
> >> level mode enter.
> >>
> >> > I mean, perhaps suspend time should include
> >> > device suspend?
> >> That makes sense. We have a few options here:
> >> 1) keep the traces as proposed to trace the low level machine code only,
> >> 2) move the traces to the entry and exit of suspend_enter so that it
> >> includes the prepare and late_prepare (+ the associated wake-up)
> >> callbacks as well,
> >> 3) move the traces to suspend_devices_and_enter so that it includes 2)
> >> and the handling of the console and the devices,
> >> 4) move the traces to enter_state do that it includes 3), the call to
> >> sys_sync and the user space freeze.
> >>
> >> Note that the the SNAPSHOT_2RAM ioctl code also calls
> >> suspend_devices_and_enter, so if only 4) is used no trace will be
> >> generated in that case.
> >>
> >> I am in favor of 3) of 4).
> >> What do you think?
> >
> > Why don't we keep the tracepoints as proposed _and_ add two additional
> > tracepoints around device suspend-resume?
> I like the idea but that requires to extend the current API with
> additional suspend tracepoints or device state change tracepoints.
> That can be done once the current API is firmly in place.
> Today the only trace API for suspend is machine_suspend(unsigned int
> state), so I think the best option is 3) here above.
> 
> Unless there is an objection I am pushing 3) asap.

Fine by me.

Thanks,
Rafael

Re: [PATCH 1/3] perf: add calls to suspend trace point

[Pavel Machek]

Hi!

> > >> I am in favor of 3) of 4).
> > >> What do you think?
> > >
> > > Why don't we keep the tracepoints as proposed _and_ add two additional
> > > tracepoints around device suspend-resume?
> > I like the idea but that requires to extend the current API with
> > additional suspend tracepoints or device state change tracepoints.
> > That can be done once the current API is firmly in place.
> > Today the only trace API for suspend is machine_suspend(unsigned int
> > state), so I think the best option is 3) here above.
> > 
> > Unless there is an objection I am pushing 3) asap.
> 
> Fine by me.

Why not...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html