lkml.org 
[lkml]   [2010]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] tracing, perf: add more power related events
Date
[Dropping discuss@lesswatts.org from the CC list.]

On Wednesday, September 22, 2010, Jean Pihet wrote:
> Hi,
>
> Here is a patch that redefines the power events API. The advantages
> are: easier maintainance of the kernel and the
> user space tools, a cleaner and more generic interface, more
> parameters for fine tracing and even documentation!
>
> Thomas, this patch has your patch above merged in ('power-trace: Use
> power_switch_state instead of power_start and power_end'). The revised
> ACPI patch is coming asap.
>
> The trace points for x86 and OMAP are also udated accordingly.
>
> The pytimechart tool needs an update for the new API. This can be done
> as soon as the kernel code gets merged in.
> Please note the point below about the existing code in builtin-timechart.c.
>
> On Sat, Sep 18, 2010 at 12:26 AM, Thomas Renninger <trenn@suse.de> wrote:
> > On Friday 17 September 2010 18:24:12 Ingo Molnar wrote:
> >>
> >> * Thomas Renninger <trenn@suse.de> wrote:
> >>
> >> > On Friday 17 September 2010 16:24:59 Ingo Molnar wrote:
> >
> >> [ You dont even have to document it, as good code is self-explanatory ;-) ]
> > I recently posted a patch exporting some things through /sys/kernel/debug/...
> > Greg complained that a file for Documentation/ABI/{testing,stable}/* is missing
> > and I fully agree.
> The proposed patch has the documentation in
> Documentation/trace/events-power.txt.
>
> > If different userspace apps should make use of this (in above case nobody
> > than my debug userspace tool will...) and this should be called something like an API,
> > it should be documented and if something changes, it should
> > first be marked deprecated, etc.
> Note: the exsiting code in tools/perf/builtin-timechart.c needs an
> update for the new events API. Is this code still maintained? I not,
> could pytimechart be merged in instead?
>
> Feedback is welcome!
>
> >
> > Thomas
> >
>
> Thanks,
> Jean
>
> ---
> From f37d1875050d33273b10e99497b4059b37e6680d Mon Sep 17 00:00:00 2001
> From: Jean Pihet <jean.pihet@newoldbits.com>
> Date: Wed, 22 Sep 2010 17:10:47 +0200
> Subject: [PATCH] tools, perf: redefine the power events API
>
> Redefine the API with:
> - power_switch_state for C-, P- and S-states,
> - clock and power_domain events
>
> The new API allows easier maintainance of the kernel and the
> user space tools, a cleaner and more generic interface,
> more parameters for fine tracing and even documentation.
>
> The new events are used by the x86 and OMAP platforms.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
> Documentation/trace/events-power.txt | 70 +++++++++++++++++++++++++
> arch/arm/mach-omap2/cpuidle34xx.c | 3 +
> arch/arm/mach-omap2/pm34xx.c | 11 ++++
> arch/arm/mach-omap2/powerdomain.c | 3 +
> arch/arm/plat-omap/clock.c | 13 ++++-
> arch/arm/plat-omap/cpu-omap.c | 3 +
> arch/x86/kernel/process.c | 13 +++--
> arch/x86/kernel/process_32.c | 3 +-
> arch/x86/kernel/process_64.c | 3 +-
> drivers/cpufreq/cpufreq.c | 3 +-
> drivers/cpuidle/cpuidle.c | 2 +-
> drivers/idle/intel_idle.c | 2 +-
> include/trace/events/power.h | 95 ++++++++++++++++------------------
> kernel/trace/power-traces.c | 2 -
> 14 files changed, 161 insertions(+), 65 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..967f842
> --- /dev/null
> +++ b/Documentation/trace/events-power.txt
> @@ -0,0 +1,70 @@
> +
> + 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
> +============================
> +
> +power_switch_state type=%lu state=%lu sub_state=%lu 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 */
> + . POWER_SSTATE = 3, /* Suspend */

This POWER_SSTATE thing seems to be totally artificial and omap-specific.

Why do you want it to be done this way?

Or is the ACPI handling added in the ACPI patch? In which case, why don't you
put that power_switch_state(POWER_SSTATE, 1, 0, cpu) into
kernel/power/suspend.c:suspend_enter() (and analogously for
power_switch_state(POWER_SSTATE, 0, 0, cpu)).

Moreover, why is the cpu argument necessary for POWER_SSTATE at all?

Thanks,
Rafael


\
 
 \ /
  Last update: 2010-09-22 20:53    [from the cache]
©2003-2011 Jasper Spaans