[PATCH] tracing, perf : add cpu hotplug trace events

[PATCH] tracing, perf : add cpu hotplug trace events

[Vincent Guittot]

Please find below a proposal for adding new trace events for cpu
hotplug.The goal is to measure the latency of each part (kernel,
architecture and platform) and also to trace the cpu hotplug activity
with other power events. I have tested these traces events on an arm
platform

Comments are welcome.

Date: Wed, 15 Dec 2010 11:22:21 +0100
Subject: [PATCH] hotplug tracepoint

this patch adds new events for cpu hotplug tracing
* plug/unplug sequence
* architecture and machine latency measurements

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
include/trace/events/hotplug.h |   71 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
create mode 100644 include/trace/events/hotplug.h

diff --git a/include/trace/events/hotplug.h b/include/trace/events/hotplug.h
new file mode 100644
index 0000000..51c86ab
--- /dev/null
+++ b/include/trace/events/hotplug.h
@@ -0,0 +1,71 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hotplug
+
+#if !defined(_TRACE_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HOTPLUG_H
+
+#include <linux/ktime.h>
+#include <linux/tracepoint.h>
+
+#ifndef _TRACE_HOTPLUG_ENUM_
+#define _TRACE_HOTPLUG_ENUM_
+enum hotplug_type {
+ HOTPLUG_UNPLUG = 0,
+ HOTPLUG_PLUG = 1
+};
+
+enum hotplug_step {
+ HOTPLUG_KERNEL = 0,
+ HOTPLUG_ARCH = 1,
+ HOTPLUG_MACH = 2
+};
+#endif
+
+TRACE_EVENT(hotplug_start,
+
+ TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
+
+ TP_ARGS(type, step, cpuid),
+
+ TP_STRUCT__entry(
+ __field(u32, type)
+ __field(u32, step)
+ __field(u32, cpuid)
+ ),
+
+ TP_fast_assign(
+ __entry->type = type;
+ __entry->step = step;
+ __entry->cpuid = cpuid;
+ ),
+
+ TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
+ (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
+);
+
+TRACE_EVENT(hotplug_end,
+
+ TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
+
+ TP_ARGS(type, step, cpuid),
+
+ TP_STRUCT__entry(
+ __field(u32, type)
+ __field(u32, step)
+ __field(u32, cpuid)
+ ),
+
+ TP_fast_assign(
+ __entry->type = type;
+ __entry->step = step;
+ __entry->cpuid = cpuid;
+ ),
+
+ TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
+ (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
+);
+
+#endif /* _TRACE_HOTPLUG_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.7.0.4

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Amit Kucheria]

Vincent,

Adding Steve Rostedt and Frederic Weisbecker to CC list as maintainers
of the trace subsystem

Also, a sample patch against your SoC on how you plan to use this
might be useful. You can mark the patch as RFC for now.

More comments below.

On Tue, Jan 4, 2011 at 3:20 PM, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
> Please find below a proposal for adding new trace events for cpu
> hotplug.The goal is to measure the latency of each part (kernel,
> architecture and platform) and also to trace the cpu hotplug activity
> with other power events. I have tested these traces events on an arm
> platform
>
> Comments are welcome.
>
> Date: Wed, 15 Dec 2010 11:22:21 +0100
> Subject: [PATCH] hotplug tracepoint
>
> this patch adds new events for cpu hotplug tracing
> * plug/unplug sequence
> * architecture and machine latency measurements
>
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> include/trace/events/hotplug.h |   71 ++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 71 insertions(+), 0 deletions(-)
> create mode 100644 include/trace/events/hotplug.h
>
> diff --git a/include/trace/events/hotplug.h b/include/trace/events/hotplug.h
> new file mode 100644
> index 0000000..51c86ab
> --- /dev/null
> +++ b/include/trace/events/hotplug.h
> @@ -0,0 +1,71 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM hotplug
> +
> +#if !defined(_TRACE_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HOTPLUG_H
> +
> +#include <linux/ktime.h>
> +#include <linux/tracepoint.h>
> +
> +#ifndef _TRACE_HOTPLUG_ENUM_
> +#define _TRACE_HOTPLUG_ENUM_
> +enum hotplug_type {
> + HOTPLUG_UNPLUG = 0,
> + HOTPLUG_PLUG = 1
> +};
> +
> +enum hotplug_step {
> + HOTPLUG_KERNEL = 0,
> + HOTPLUG_ARCH = 1,
> + HOTPLUG_MACH = 2
> +};
> +#endif

s/HOTPLUG_KERNEL/HOTPLUG_CORE/  ?

Comments regarding what these mean would be nice. ARM has core, arch
and machine level configuration, but x86, for example might not use
machine at all.

> +
> +TRACE_EVENT(hotplug_start,
> +
> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
> +
> + TP_ARGS(type, step, cpuid),
> +
> + TP_STRUCT__entry(
> + __field(u32, type)
> + __field(u32, step)
> + __field(u32, cpuid)
> + ),
> +
> + TP_fast_assign(
> + __entry->type = type;
> + __entry->step = step;
> + __entry->cpuid = cpuid;
> + ),
> +
> + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
> + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
> +);
> +
> +TRACE_EVENT(hotplug_end,
> +
> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
> +
> + TP_ARGS(type, step, cpuid),
> +
> + TP_STRUCT__entry(
> + __field(u32, type)
> + __field(u32, step)
> + __field(u32, cpuid)
> + ),
> +
> + TP_fast_assign(
> + __entry->type = type;
> + __entry->step = step;
> + __entry->cpuid = cpuid;
> + ),
> +
> + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
> + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
> +);
> +
> +#endif /* _TRACE_HOTPLUG_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> --
> 1.7.0.4

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Vincent Guittot]

On 7 January 2011 10:21, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> Vincent,
>
> Adding Steve Rostedt and Frederic Weisbecker to CC list as maintainers
> of the trace subsystem
>
> Also, a sample patch against your SoC on how you plan to use this
> might be useful. You can mark the patch as RFC for now.
>

I'm going to prepare a sample patch based on my SoC.
The trace output with current proposal looks like below on my SoC . I
have added some comments :

# tracer: nop
#
#           TASK-PID    CPU#    TIMESTAMP  FUNCTION
#              | |       |          |         |
           bash-3603  [000]   133.012161: hotplug_start: type=1 step=0
cpuid=1     Start to plug cpu1
           bash-3603  [000]   133.032270: hotplug_start: type=1 step=1
cpuid=1     boot_secondary
        <idle>-0     [001]   133.032372: hotplug_end: type=1 step=2
cpuid=1         cpu1 is up
           bash-3603  [000]   133.032448: hotplug_end: type=1 step=1
cpuid=1      cpu1 is online
           bash-3603  [000]   133.074754: hotplug_end: type=1 step=0
cpuid=1      End of the plug sequence

> More comments below.
>
> On Tue, Jan 4, 2011 at 3:20 PM, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
>> Please find below a proposal for adding new trace events for cpu
>> hotplug.The goal is to measure the latency of each part (kernel,
>> architecture and platform) and also to trace the cpu hotplug activity
>> with other power events. I have tested these traces events on an arm
>> platform
>>
>> Comments are welcome.
>>
>> Date: Wed, 15 Dec 2010 11:22:21 +0100
>> Subject: [PATCH] hotplug tracepoint
>>
>> this patch adds new events for cpu hotplug tracing
>> * plug/unplug sequence
>> * architecture and machine latency measurements
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>> ---
>> include/trace/events/hotplug.h |   71 ++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 71 insertions(+), 0 deletions(-)
>> create mode 100644 include/trace/events/hotplug.h
>>
>> diff --git a/include/trace/events/hotplug.h b/include/trace/events/hotplug.h
>> new file mode 100644
>> index 0000000..51c86ab
>> --- /dev/null
>> +++ b/include/trace/events/hotplug.h
>> @@ -0,0 +1,71 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM hotplug
>> +
>> +#if !defined(_TRACE_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_HOTPLUG_H
>> +
>> +#include <linux/ktime.h>
>> +#include <linux/tracepoint.h>
>> +
>> +#ifndef _TRACE_HOTPLUG_ENUM_
>> +#define _TRACE_HOTPLUG_ENUM_
>> +enum hotplug_type {
>> + HOTPLUG_UNPLUG = 0,
>> + HOTPLUG_PLUG = 1
>> +};
>> +
>> +enum hotplug_step {
>> + HOTPLUG_KERNEL = 0,
>> + HOTPLUG_ARCH = 1,
>> + HOTPLUG_MACH = 2
>> +};
>> +#endif
>
> s/HOTPLUG_KERNEL/HOTPLUG_CORE/  ?
>

ok, I'm going to change for HOTPLUG_CORE

> Comments regarding what these mean would be nice. ARM has core, arch
> and machine level configuration, but x86, for example might not use
> machine at all.
>

ok, I'm going to add comments

The hotplug_step are used to define which kind of code is running.
-The HOTPLUG_CORE refers to code that is used whatever the system is.
-The HOTPLUG_ARCH refers to code that is dedicated to an architecture.
Typically, the 3 functions:
__cpu_up(), __cpu_disable() and __cpu_die().
-The HOTPLUG_MACH is used by architectures which have some dedicated
SW for shutting down the cpu like on Arm SoC. For the Arm
architecture, the machine step is linked to the platform_cpu_die
function.

>> +
>> +TRACE_EVENT(hotplug_start,
>> +
>> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
>> +
>> + TP_ARGS(type, step, cpuid),
>> +
>> + TP_STRUCT__entry(
>> + __field(u32, type)
>> + __field(u32, step)
>> + __field(u32, cpuid)
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->type = type;
>> + __entry->step = step;
>> + __entry->cpuid = cpuid;
>> + ),
>> +
>> + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
>> + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
>> +);
>> +
>> +TRACE_EVENT(hotplug_end,
>> +
>> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
>> +
>> + TP_ARGS(type, step, cpuid),
>> +
>> + TP_STRUCT__entry(
>> + __field(u32, type)
>> + __field(u32, step)
>> + __field(u32, cpuid)
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->type = type;
>> + __entry->step = step;
>> + __entry->cpuid = cpuid;
>> + ),
>> +
>> + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
>> + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
>> +);
>> +
>> +#endif /* _TRACE_HOTPLUG_H */
>> +
>> +/* This part must be outside protection */
>> +#include <trace/define_trace.h>
>> --
>> 1.7.0.4
>

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Frederic Weisbecker]

On Tue, Jan 04, 2011 at 10:50:36AM +0100, Vincent Guittot wrote:
> Please find below a proposal for adding new trace events for cpu
> hotplug.The goal is to measure the latency of each part (kernel,
> architecture and platform) and also to trace the cpu hotplug activity
> with other power events. I have tested these traces events on an arm
> platform
> 
> Comments are welcome.
> 
> Date: Wed, 15 Dec 2010 11:22:21 +0100
> Subject: [PATCH] hotplug tracepoint
> 
> this patch adds new events for cpu hotplug tracing
> * plug/unplug sequence
> * architecture and machine latency measurements
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> include/trace/events/hotplug.h |   71 ++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 71 insertions(+), 0 deletions(-)
> create mode 100644 include/trace/events/hotplug.h
> 
> diff --git a/include/trace/events/hotplug.h b/include/trace/events/hotplug.h
> new file mode 100644
> index 0000000..51c86ab
> --- /dev/null
> +++ b/include/trace/events/hotplug.h
> @@ -0,0 +1,71 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM hotplug
> +
> +#if !defined(_TRACE_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HOTPLUG_H
> +
> +#include <linux/ktime.h>
> +#include <linux/tracepoint.h>
> +
> +#ifndef _TRACE_HOTPLUG_ENUM_
> +#define _TRACE_HOTPLUG_ENUM_
> +enum hotplug_type {
> + HOTPLUG_UNPLUG = 0,
> + HOTPLUG_PLUG = 1
> +};
> +
> +enum hotplug_step {
> + HOTPLUG_KERNEL = 0,
> + HOTPLUG_ARCH = 1,
> + HOTPLUG_MACH = 2
> +};
> +#endif
> +
> +TRACE_EVENT(hotplug_start,

hotplug is way too generic.

cpu_hotplug may be?

> +
> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),

I feel a bit uncomfortable with these opaque type and step.

What about splitting the events:

	cpu_down_start
	cpu_down_end

	cpu_up_start
	cpu_up_end

This ways they are much more self-explanatory.

I also feel uncomfortable about exposing arch step details in core
tracepoints.

But if we consider the following sequence:

	cpu_down() {
		__cpu_disable() {
			platform_cpu_disable();
		}
	}

Then exposing start/end of cpu_disable() makes sense, by way of:

	cpu_arch_disable_start
        cpu_arch_disable_end

        cpu_arch_enable_start
        cpu_arch_enable_end


	cpu_arch_die_start
        cpu_arch_die_end

        cpu_arch_die_start
        cpu_arch_die_end

Because they are arch events that you can retrieve everywhere, the tracepoints
are still called from the code code.

Now for the machine part, it's very highly arch specific, most notably for arm
so I wonder if it would make more sense to keep that seperate into arch
tracepoints.

How does that all look? I hope I'm not overengineering.

Creating such series of similar tracepoints is quite easy and quick using
DECLARE_EVENT_CLASS and DEFINE_EVENT.

> +
> + TP_ARGS(type, step, cpuid),
> +
> + TP_STRUCT__entry(
> + __field(u32, type)
> + __field(u32, step)
> + __field(u32, cpuid)
> + ),

And please use a proper indentation for trace_events.
You can have a look at the examples in include/trace/events/

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Steven Rostedt]

On Fri, 2011-01-07 at 14:51 +0530, Amit Kucheria wrote:

> > +
> > +TRACE_EVENT(hotplug_start,
> > +
> > + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
> > +
> > + TP_ARGS(type, step, cpuid),
> > +
> > + TP_STRUCT__entry(
> > + __field(u32, type)
> > + __field(u32, step)
> > + __field(u32, cpuid)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->type = type;
> > + __entry->step = step;
> > + __entry->cpuid = cpuid;
> > + ),
> > +
> > + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
> > + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
> > +);
> > +
> > +TRACE_EVENT(hotplug_end,
> > +
> > + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
> > +
> > + TP_ARGS(type, step, cpuid),
> > +
> > + TP_STRUCT__entry(
> > + __field(u32, type)
> > + __field(u32, step)
> > + __field(u32, cpuid)
> > + ),
> > +
> > + TP_fast_assign(
> > + __entry->type = type;
> > + __entry->step = step;
> > + __entry->cpuid = cpuid;
> > + ),
> > +
> > + TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
> > + (unsigned long)__entry->step, (unsigned long)__entry->cpuid)
> > +);
> > +
> > 

Please use classes when having tracepoints that have the same fields.
This will save a bit of kernel memory. Something like:


DECLARE_EVENT_CLASS(hotplug_template,

	TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),

	TP_ARGS(type, step, cpuid),

	TP_STRUCT__entry(
		__field(u32, type)
		__field(u32, step)
		__field(u32, cpuid)
	),

	 TP_fast_assign(
		__entry->type = type;
		__entry->step = step;
		__entry->cpuid = cpuid;
	),

	TP_printk("type=%lu step=%lu cpuid=%lu", (unsigned long)__entry->type,
		(unsigned long)__entry->step, (unsigned long)__entry->cpuid)
);

DEFINE_EVENT(hotplug_template, hotplug_start,
	TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
	TP_ARGS(type, step, cpuid);

DEFINE_EVENT(hotplug_template, hotplug_end,
	TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
	TP_ARGS(type, step, cpuid);

-- Steve

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Vincent Guittot]

On 7 January 2011 16:12, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Tue, Jan 04, 2011 at 10:50:36AM +0100, Vincent Guittot wrote:
>> Please find below a proposal for adding new trace events for cpu
>> hotplug.The goal is to measure the latency of each part (kernel,
>> architecture and platform) and also to trace the cpu hotplug activity
>> with other power events. I have tested these traces events on an arm
>> platform
>>
>> Comments are welcome.
>>
>> Date: Wed, 15 Dec 2010 11:22:21 +0100
>> Subject: [PATCH] hotplug tracepoint
>>
>> this patch adds new events for cpu hotplug tracing
>> * plug/unplug sequence
>> * architecture and machine latency measurements
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>> ---
>> include/trace/events/hotplug.h |   71 ++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 71 insertions(+), 0 deletions(-)
>> create mode 100644 include/trace/events/hotplug.h
>>
>> diff --git a/include/trace/events/hotplug.h b/include/trace/events/hotplug.h
>> new file mode 100644
>> index 0000000..51c86ab
>> --- /dev/null
>> +++ b/include/trace/events/hotplug.h
>> @@ -0,0 +1,71 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM hotplug
>> +
>> +#if !defined(_TRACE_HOTPLUG_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_HOTPLUG_H
>> +
>> +#include <linux/ktime.h>
>> +#include <linux/tracepoint.h>
>> +
>> +#ifndef _TRACE_HOTPLUG_ENUM_
>> +#define _TRACE_HOTPLUG_ENUM_
>> +enum hotplug_type {
>> + HOTPLUG_UNPLUG = 0,
>> + HOTPLUG_PLUG = 1
>> +};
>> +
>> +enum hotplug_step {
>> + HOTPLUG_KERNEL = 0,
>> + HOTPLUG_ARCH = 1,
>> + HOTPLUG_MACH = 2
>> +};
>> +#endif
>> +
>> +TRACE_EVENT(hotplug_start,
>
> hotplug is way too generic.
>
> cpu_hotplug may be?
>

that's fine for me

>> +
>> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
>
> I feel a bit uncomfortable with these opaque type and step.
>
> What about splitting the events:
>
>        cpu_down_start
>        cpu_down_end
>
>        cpu_up_start
>        cpu_up_end
>
> This ways they are much more self-explanatory.
>
> I also feel uncomfortable about exposing arch step details in core
> tracepoints.
>
> But if we consider the following sequence:
>
>        cpu_down() {
>                __cpu_disable() {
>                        platform_cpu_disable();
>                }
>        }
>
> Then exposing start/end of cpu_disable() makes sense, by way of:
>
>        cpu_arch_disable_start
>        cpu_arch_disable_end
>
>        cpu_arch_enable_start
>        cpu_arch_enable_end
>
>
>        cpu_arch_die_start
>        cpu_arch_die_end
>
>        cpu_arch_die_start
>        cpu_arch_die_end
>
> Because they are arch events that you can retrieve everywhere, the tracepoints
> are still called from the code code.
>
> Now for the machine part, it's very highly arch specific, most notably for arm
> so I wonder if it would make more sense to keep that seperate into arch
> tracepoints.
>

May be we could find some event names that matches for all system and
that can be kept in the same file ?

> How does that all look? I hope I'm not overengineering.
>

that's could be ok for me, I can find almost the same kind of
information with this solution. I just wonder what traces are the
easiest to treat for extracting some latency measurement or to treat
with other event like the power event.

> Creating such series of similar tracepoints is quite easy and quick using
> DECLARE_EVENT_CLASS and DEFINE_EVENT.
>

ok

>> +
>> + TP_ARGS(type, step, cpuid),
>> +
>> + TP_STRUCT__entry(
>> + __field(u32, type)
>> + __field(u32, step)
>> + __field(u32, cpuid)
>> + ),
>
> And please use a proper indentation for trace_events.
> You can have a look at the examples in include/trace/events/
>

ok, i will take example from include/trace/events/

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Frederic Weisbecker]

On Fri, Jan 07, 2011 at 07:25:08PM +0100, Vincent Guittot wrote:
> On 7 January 2011 16:12, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> +
> >> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
> >
> > I feel a bit uncomfortable with these opaque type and step.
> >
> > What about splitting the events:
> >
> >        cpu_down_start
> >        cpu_down_end
> >
> >        cpu_up_start
> >        cpu_up_end
> >
> > This ways they are much more self-explanatory.
> >
> > I also feel uncomfortable about exposing arch step details in core
> > tracepoints.
> >
> > But if we consider the following sequence:
> >
> >        cpu_down() {
> >                __cpu_disable() {
> >                        platform_cpu_disable();
> >                }
> >        }
> >
> > Then exposing start/end of cpu_disable() makes sense, by way of:
> >
> >        cpu_arch_disable_start
> >        cpu_arch_disable_end
> >
> >        cpu_arch_enable_start
> >        cpu_arch_enable_end
> >
> >
> >        cpu_arch_die_start
> >        cpu_arch_die_end
> >
> >        cpu_arch_die_start
> >        cpu_arch_die_end
> >
> > Because they are arch events that you can retrieve everywhere, the tracepoints
> > are still called from the code code.
> >
> > Now for the machine part, it's very highly arch specific, most notably for arm
> > so I wonder if it would make more sense to keep that seperate into arch
> > tracepoints.
> >
> 
> May be we could find some event names that matches for all system and
> that can be kept in the same file ?

But that's only an ARM concern, right? So ARM can create its own
set of tracepoints for that. If that becomes more widely useful then
we can think about gathering the whole into a single file.

> > How does that all look? I hope I'm not overengineering.
> >
> 
> that's could be ok for me, I can find almost the same kind of
> information with this solution. I just wonder what traces are the
> easiest to treat for extracting some latency measurement or to treat
> with other event like the power event.

Hmm, I'm not sure what you mean. You want to know which tracepoints
can be useful to measure latencies? Well, it depends on what kind
of latency you seek in general: scheduler, io, etc...

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Vincent Guittot]

On 14 January 2011 12:35, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Fri, Jan 07, 2011 at 07:25:08PM +0100, Vincent Guittot wrote:
>> On 7 January 2011 16:12, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> >> +
>> >> + TP_PROTO(unsigned int type, unsigned int step, unsigned int cpuid),
>> >
>> > I feel a bit uncomfortable with these opaque type and step.
>> >
>> > What about splitting the events:
>> >
>> >        cpu_down_start
>> >        cpu_down_end
>> >
>> >        cpu_up_start
>> >        cpu_up_end
>> >
>> > This ways they are much more self-explanatory.
>> >
>> > I also feel uncomfortable about exposing arch step details in core
>> > tracepoints.
>> >
>> > But if we consider the following sequence:
>> >
>> >        cpu_down() {
>> >                __cpu_disable() {
>> >                        platform_cpu_disable();
>> >                }
>> >        }
>> >
>> > Then exposing start/end of cpu_disable() makes sense, by way of:
>> >
>> >        cpu_arch_disable_start
>> >        cpu_arch_disable_end
>> >
>> >        cpu_arch_enable_start
>> >        cpu_arch_enable_end
>> >
>> >
>> >        cpu_arch_die_start
>> >        cpu_arch_die_end
>> >
>> >        cpu_arch_die_start
>> >        cpu_arch_die_end
>> >
>> > Because they are arch events that you can retrieve everywhere, the tracepoints
>> > are still called from the code code.
>> >
>> > Now for the machine part, it's very highly arch specific, most notably for arm
>> > so I wonder if it would make more sense to keep that seperate into arch
>> > tracepoints.
>> >
>>
>> May be we could find some event names that matches for all system and
>> that can be kept in the same file ?
>
> But that's only an ARM concern, right? So ARM can create its own
> set of tracepoints for that. If that becomes more widely useful then
> we can think about gathering the whole into a single file.
>

OK, we can do like that

>> > How does that all look? I hope I'm not overengineering.
>> >
>>
>> that's could be ok for me, I can find almost the same kind of
>> information with this solution. I just wonder what traces are the
>> easiest to treat for extracting some latency measurement or to treat
>> with other event like the power event.
>
> Hmm, I'm not sure what you mean. You want to know which tracepoints
> can be useful to measure latencies? Well, it depends on what kind
> of latency you seek in general: scheduler, io, etc...
>

I was just wondering which tracepoints format between my 1st proposal
and yours was the easier to post process by an application like
pytimechart.

I have updated the cpu hotplug tracepoint according to your remarks
and steve's ones. I have just replaced the second
cpu_arch_die_start/end in your proposal by cpu_arch_dead_start/endfrq

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Frederic Weisbecker]

On Mon, Jan 17, 2011 at 07:49:58AM -0600, Vincent Guittot wrote:
> I was just wondering which tracepoints format between my 1st proposal
> and yours was the easier to post process by an application like
> pytimechart.

No idea as pytimechart uses his own ad hoc event parsing. Either
way there won't be much differences though.

> I have updated the cpu hotplug tracepoint according to your remarks
> and steve's ones. I have just replaced the second
> cpu_arch_die_start/end in your proposal by cpu_arch_dead_start/endfrq

Tracepoints tend to describe actions rather than states, although I can
show you some exceptions as well. But this tends to be the major
tendency. I suggest you to be stay consistent with this scheme.

Thanks.

Re: [PATCH] tracing, perf : add cpu hotplug trace events

[Vincent Guittot]

On 17 January 2011 10:21, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Mon, Jan 17, 2011 at 07:49:58AM -0600, Vincent Guittot wrote:
>> I was just wondering which tracepoints format between my 1st proposal
>> and yours was the easier to post process by an application like
>> pytimechart.
>
> No idea as pytimechart uses his own ad hoc event parsing. Either
> way there won't be much differences though.
>
>> I have updated the cpu hotplug tracepoint according to your remarks
>> and steve's ones. I have just replaced the second
>> cpu_arch_die_start/end in your proposal by cpu_arch_dead_start/endfrq
>
> Tracepoints tend to describe actions rather than states, although I can
> show you some exceptions as well. But this tends to be the major
> tendency. I suggest you to be stay consistent with this scheme.
>

For the cpu_die function which is called in the idle loop, we use
cpu_hotplug_arch_die_start/end
and for the __cpu_die function which waits for the cpu death, we could
use cpu_hotplug_arch_wait_death_start/end ?

as a summary we have :

cpu_hotplug_up_start/end
cpu_hotplug_down_start/end
cpu_hotplug_arch_up_start/end
cpu_hotplug_arch_disable_start/end
cpu_hotplug_arch_die_start/end
cpu_hotplug_arch_wait_death_start/end


> Thanks.
>