* Re: [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu()
@ 2013-10-09 20:08 Sergey Senozhatsky
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Senozhatsky @ 2013-10-09 20:08 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 427 bytes --]
On (10/09/13 12:39), Kristen Carlson Accardi wrote:
> > On (10/09/13 11:54), Shaojie Sun wrote:
> > > Hi Sergey
> > > I tested the patch running in our system.
> > > Next is the test result.
> >
> > Hello,
> > So the patch seems to work for you. Thanks.
> >
> > -ss
>
> So does this mean you want this patch applied?
>
I think it's already applied - af8431eb0062f9f785ec593f2e6747c75089962d
-ss
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu()
@ 2013-10-09 19:39 Kristen Carlson Accardi
0 siblings, 0 replies; 5+ messages in thread
From: Kristen Carlson Accardi @ 2013-10-09 19:39 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 363 bytes --]
On Wed, 9 Oct 2013 12:02:17 +0300
Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com> wrote:
> On (10/09/13 11:54), Shaojie Sun wrote:
> > Hi Sergey
> > I tested the patch running in our system.
> > Next is the test result.
>
> Hello,
> So the patch seems to work for you. Thanks.
>
> -ss
So does this mean you want this patch applied?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu()
@ 2013-10-09 9:02 Sergey Senozhatsky
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Senozhatsky @ 2013-10-09 9:02 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4854 bytes --]
On (10/09/13 11:54), Shaojie Sun wrote:
> Hi Sergey
> I tested the patch running in our system.
> Next is the test result.
Hello,
So the patch seems to work for you. Thanks.
-ss
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 3
> fixed [cpu_frequency] cpuid from 2 to 4
> fixed [cpu_frequency] cpuid from 2 to 0
> fixed [cpu_frequency] cpuid from 2 to 1
> fixed [cpu_frequency] cpuid from 2 to 0
>
> On Tue, Oct 8, 2013 at 10:55 PM, Sergey Senozhatsky
> <sergey.senozhatsky(a)gmail.com> wrote:
> > Could you please test this patch?
> >
> >
> > struct perf_sample's trace cpu is a raw_smp_processor_id()
> > (as requsted by PERF_SAMPLE_CPU) by the time of perf_event_output()
> > call, which may differ from original struct perf_event's cpu.
> > thus we need to fix sample->trace.cpu via fixup_sample_trace_cpu()
> > call.
> >
> > debugging output has demonstrated that some events (namely cpu_frequency)
> > were routed incorrectly (sample->trace.cpu != pevent_get_field_val("cpu_id")):
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 2
> > fix [cpu_frequency] cpu_nr from 1 to 2
> > fix [cpu_frequency] cpu_nr from 1 to 3
> > fix [cpu_frequency] cpu_nr from 1 to 3
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 2
> > fix [cpu_frequency] cpu_nr from 1 to 2
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 0
> > fix [cpu_frequency] cpu_nr from 1 to 0
> >
> > Thanks for reporting and tracking this down to (not in any particular
> > order): Shaojie Sun, Jon Medhurst, Sanjay Rawat, SunShaoJie.
> >
> > Cc: Shaojie Sun <shaojie.sun(a)linaro.org>
> > Cc: Jon Medhurst <jon.medhurst(a)linaro.org>
> > Cc: Sanjay Rawat <sanjay.rawat(a)linaro.org>
> > Cc: SunShaoJie <sunshaojie(a)huawei.com>
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
> >
> > ---
> >
> > diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> > index 8d480e8..b0e982b 100644
> > --- a/src/perf/perf_bundle.cpp
> > +++ b/src/perf/perf_bundle.cpp
> > @@ -285,6 +285,31 @@ static bool event_sort_function (void *i, void *j)
> > return (timestamp(I)<timestamp(J));
> > }
> >
> > +/*
> > + * sample's PERF_SAMPLE_CPU cpu nr is a raw_smp_processor_id() by the
> > + * time of perf_event_output(), which may differ from struct perf_event
> > + * cpu, thus we need to fix sample->trace.cpu.
> > + */
> > +static void fixup_sample_trace_cpu(struct perf_sample *sample)
> > +{
> > + struct event_format *event;
> > + struct pevent_record rec;
> > + unsigned long long cpu_nr;
> > + int type;
> > + int ret;
> > +
> > + rec.data = &sample->data;
> > + type = pevent_data_type(perf_event::pevent, &rec);
> > + event = pevent_find_event(perf_event::pevent, type);
> > + if (!event)
> > + return;
> > + /** don't touch trace if event does not contain cpu_id field*/
> > + ret = pevent_get_field_val(NULL, event, "cpu_id", &rec, &cpu_nr, 0);
> > + if (ret < 0)
> > + return;
> > + sample->trace.cpu = cpu_nr;
> > +}
> > +
> > void perf_bundle::process(void)
> > {
> > unsigned int i;
> > @@ -309,6 +334,7 @@ void perf_bundle::process(void)
> > if (sample->header.type != PERF_RECORD_SAMPLE)
> > continue;
> >
> > + fixup_sample_trace_cpu(sample);
> > handle_trace_point(&sample->data, sample->trace.cpu, sample->trace.time);
> > }
> > }
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu()
@ 2013-10-09 3:54 Shaojie Sun
0 siblings, 0 replies; 5+ messages in thread
From: Shaojie Sun @ 2013-10-09 3:54 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4510 bytes --]
Hi Sergey
I tested the patch running in our system.
Next is the test result.
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 3
fixed [cpu_frequency] cpuid from 2 to 4
fixed [cpu_frequency] cpuid from 2 to 0
fixed [cpu_frequency] cpuid from 2 to 1
fixed [cpu_frequency] cpuid from 2 to 0
On Tue, Oct 8, 2013 at 10:55 PM, Sergey Senozhatsky
<sergey.senozhatsky(a)gmail.com> wrote:
> Could you please test this patch?
>
>
> struct perf_sample's trace cpu is a raw_smp_processor_id()
> (as requsted by PERF_SAMPLE_CPU) by the time of perf_event_output()
> call, which may differ from original struct perf_event's cpu.
> thus we need to fix sample->trace.cpu via fixup_sample_trace_cpu()
> call.
>
> debugging output has demonstrated that some events (namely cpu_frequency)
> were routed incorrectly (sample->trace.cpu != pevent_get_field_val("cpu_id")):
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 2
> fix [cpu_frequency] cpu_nr from 1 to 2
> fix [cpu_frequency] cpu_nr from 1 to 3
> fix [cpu_frequency] cpu_nr from 1 to 3
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 2
> fix [cpu_frequency] cpu_nr from 1 to 2
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 0
> fix [cpu_frequency] cpu_nr from 1 to 0
>
> Thanks for reporting and tracking this down to (not in any particular
> order): Shaojie Sun, Jon Medhurst, Sanjay Rawat, SunShaoJie.
>
> Cc: Shaojie Sun <shaojie.sun(a)linaro.org>
> Cc: Jon Medhurst <jon.medhurst(a)linaro.org>
> Cc: Sanjay Rawat <sanjay.rawat(a)linaro.org>
> Cc: SunShaoJie <sunshaojie(a)huawei.com>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
>
> ---
>
> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> index 8d480e8..b0e982b 100644
> --- a/src/perf/perf_bundle.cpp
> +++ b/src/perf/perf_bundle.cpp
> @@ -285,6 +285,31 @@ static bool event_sort_function (void *i, void *j)
> return (timestamp(I)<timestamp(J));
> }
>
> +/*
> + * sample's PERF_SAMPLE_CPU cpu nr is a raw_smp_processor_id() by the
> + * time of perf_event_output(), which may differ from struct perf_event
> + * cpu, thus we need to fix sample->trace.cpu.
> + */
> +static void fixup_sample_trace_cpu(struct perf_sample *sample)
> +{
> + struct event_format *event;
> + struct pevent_record rec;
> + unsigned long long cpu_nr;
> + int type;
> + int ret;
> +
> + rec.data = &sample->data;
> + type = pevent_data_type(perf_event::pevent, &rec);
> + event = pevent_find_event(perf_event::pevent, type);
> + if (!event)
> + return;
> + /** don't touch trace if event does not contain cpu_id field*/
> + ret = pevent_get_field_val(NULL, event, "cpu_id", &rec, &cpu_nr, 0);
> + if (ret < 0)
> + return;
> + sample->trace.cpu = cpu_nr;
> +}
> +
> void perf_bundle::process(void)
> {
> unsigned int i;
> @@ -309,6 +334,7 @@ void perf_bundle::process(void)
> if (sample->header.type != PERF_RECORD_SAMPLE)
> continue;
>
> + fixup_sample_trace_cpu(sample);
> handle_trace_point(&sample->data, sample->trace.cpu, sample->trace.time);
> }
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu()
@ 2013-10-08 14:55 Sergey Senozhatsky
0 siblings, 0 replies; 5+ messages in thread
From: Sergey Senozhatsky @ 2013-10-08 14:55 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]
Could you please test this patch?
struct perf_sample's trace cpu is a raw_smp_processor_id()
(as requsted by PERF_SAMPLE_CPU) by the time of perf_event_output()
call, which may differ from original struct perf_event's cpu.
thus we need to fix sample->trace.cpu via fixup_sample_trace_cpu()
call.
debugging output has demonstrated that some events (namely cpu_frequency)
were routed incorrectly (sample->trace.cpu != pevent_get_field_val("cpu_id")):
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 2
fix [cpu_frequency] cpu_nr from 1 to 2
fix [cpu_frequency] cpu_nr from 1 to 3
fix [cpu_frequency] cpu_nr from 1 to 3
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 2
fix [cpu_frequency] cpu_nr from 1 to 2
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 0
fix [cpu_frequency] cpu_nr from 1 to 0
Thanks for reporting and tracking this down to (not in any particular
order): Shaojie Sun, Jon Medhurst, Sanjay Rawat, SunShaoJie.
Cc: Shaojie Sun <shaojie.sun(a)linaro.org>
Cc: Jon Medhurst <jon.medhurst(a)linaro.org>
Cc: Sanjay Rawat <sanjay.rawat(a)linaro.org>
Cc: SunShaoJie <sunshaojie(a)huawei.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
---
diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
index 8d480e8..b0e982b 100644
--- a/src/perf/perf_bundle.cpp
+++ b/src/perf/perf_bundle.cpp
@@ -285,6 +285,31 @@ static bool event_sort_function (void *i, void *j)
return (timestamp(I)<timestamp(J));
}
+/*
+ * sample's PERF_SAMPLE_CPU cpu nr is a raw_smp_processor_id() by the
+ * time of perf_event_output(), which may differ from struct perf_event
+ * cpu, thus we need to fix sample->trace.cpu.
+ */
+static void fixup_sample_trace_cpu(struct perf_sample *sample)
+{
+ struct event_format *event;
+ struct pevent_record rec;
+ unsigned long long cpu_nr;
+ int type;
+ int ret;
+
+ rec.data = &sample->data;
+ type = pevent_data_type(perf_event::pevent, &rec);
+ event = pevent_find_event(perf_event::pevent, type);
+ if (!event)
+ return;
+ /** don't touch trace if event does not contain cpu_id field*/
+ ret = pevent_get_field_val(NULL, event, "cpu_id", &rec, &cpu_nr, 0);
+ if (ret < 0)
+ return;
+ sample->trace.cpu = cpu_nr;
+}
+
void perf_bundle::process(void)
{
unsigned int i;
@@ -309,6 +334,7 @@ void perf_bundle::process(void)
if (sample->header.type != PERF_RECORD_SAMPLE)
continue;
+ fixup_sample_trace_cpu(sample);
handle_trace_point(&sample->data, sample->trace.cpu, sample->trace.time);
}
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-09 20:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-09 20:08 [Powertop] [PATCH] perf_bundle: introduce fixup_sample_trace_cpu() Sergey Senozhatsky
-- strict thread matches above, loose matches on Subject: below --
2013-10-09 19:39 Kristen Carlson Accardi
2013-10-09 9:02 Sergey Senozhatsky
2013-10-09 3:54 Shaojie Sun
2013-10-08 14:55 Sergey Senozhatsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.