| Message ID | 20211201123334.679131-3-german.gomez@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support register names from all architectures | expand |
> On 01-Dec-2021, at 6:03 PM, German Gomez <german.gomez@arm.com> wrote: > > Include a "struct machine*" parameter to the process_event callback in > the scripting layer. This will allow access to the perf_env from within > this callback. > > Followup patches will build on top of this to report the correct name of > the registers in a perf.data file, consistently with the architecture > the file was recorded in. > > Signed-off-by: German Gomez <german.gomez@arm.com> > --- > tools/perf/builtin-script.c | 2 +- > .../util/scripting-engines/trace-event-perl.c | 3 ++- > .../scripting-engines/trace-event-python.c | 23 +++++++++++-------- > tools/perf/util/trace-event-scripting.c | 3 ++- > tools/perf/util/trace-event.h | 3 ++- > 5 files changed, 21 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 9434367af..711132f0b 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -2256,7 +2256,7 @@ static int process_sample_event(struct perf_tool *tool, > thread__resolve(al.thread, &addr_al, sample); > addr_al_ptr = &addr_al; > } > - scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr); > + scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr, machine); Hi, Looks like the patch is using “machine” to allow access to perf_env__arch and there by to get the “arch” value. But can we use from evsel, like "perf_env__arch(evsel__env(evsel))” to get arch value instead of including new parameter for “struct machine” ? Thanks Athira > } else { > process_event(scr, sample, evsel, &al, &addr_al, machine); > } > diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c > index 32a721b3e..6017c4660 100644 > --- a/tools/perf/util/scripting-engines/trace-event-perl.c > +++ b/tools/perf/util/scripting-engines/trace-event-perl.c > @@ -454,7 +454,8 @@ static void perl_process_event(union perf_event *event, > struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > - struct addr_location *addr_al) > + struct addr_location *addr_al, > + struct machine *machine __maybe_unused) > { > scripting_context__update(scripting_context, event, sample, evsel, al, addr_al); > perl_process_tracepoint(sample, evsel, al); > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c > index c0c010350..e164f8d00 100644 > --- a/tools/perf/util/scripting-engines/trace-event-python.c > +++ b/tools/perf/util/scripting-engines/trace-event-python.c > @@ -708,7 +708,8 @@ static void regs_map(struct regs_dump *regs, uint64_t mask, char *bf, int size) > > static void set_regs_in_dict(PyObject *dict, > struct perf_sample *sample, > - struct evsel *evsel) > + struct evsel *evsel, > + struct machine *machine __maybe_unused) > { > struct perf_event_attr *attr = &evsel->core.attr; > > @@ -776,6 +777,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > struct addr_location *addr_al, > + struct machine *machine, > PyObject *callchain) > { > PyObject *dict, *dict_sample, *brstack, *brstacksym; > @@ -849,7 +851,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, > PyLong_FromUnsignedLongLong(sample->cyc_cnt)); > } > > - set_regs_in_dict(dict, sample, evsel); > + set_regs_in_dict(dict, sample, evsel, machine); > > return dict; > } > @@ -857,7 +859,8 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, > static void python_process_tracepoint(struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > - struct addr_location *addr_al) > + struct addr_location *addr_al, > + struct machine *machine) > { > struct tep_event *event = evsel->tp_format; > PyObject *handler, *context, *t, *obj = NULL, *callchain; > @@ -964,7 +967,7 @@ static void python_process_tracepoint(struct perf_sample *sample, > PyTuple_SetItem(t, n++, dict); > > if (get_argument_count(handler) == (int) n + 1) { > - all_entries_dict = get_perf_sample_dict(sample, evsel, al, addr_al, > + all_entries_dict = get_perf_sample_dict(sample, evsel, al, addr_al, machine, > callchain); > PyTuple_SetItem(t, n++, all_entries_dict); > } else { > @@ -1366,7 +1369,8 @@ static int python_process_call_return(struct call_return *cr, u64 *parent_db_id, > static void python_process_general_event(struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > - struct addr_location *addr_al) > + struct addr_location *addr_al, > + struct machine *machine) > { > PyObject *handler, *t, *dict, *callchain; > static char handler_name[64]; > @@ -1388,7 +1392,7 @@ static void python_process_general_event(struct perf_sample *sample, > > /* ip unwinding */ > callchain = python_process_callchain(sample, evsel, al); > - dict = get_perf_sample_dict(sample, evsel, al, addr_al, callchain); > + dict = get_perf_sample_dict(sample, evsel, al, addr_al, machine, callchain); > > PyTuple_SetItem(t, n++, dict); > if (_PyTuple_Resize(&t, n) == -1) > @@ -1403,7 +1407,8 @@ static void python_process_event(union perf_event *event, > struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > - struct addr_location *addr_al) > + struct addr_location *addr_al, > + struct machine *machine) > { > struct tables *tables = &tables_global; > > @@ -1411,14 +1416,14 @@ static void python_process_event(union perf_event *event, > > switch (evsel->core.attr.type) { > case PERF_TYPE_TRACEPOINT: > - python_process_tracepoint(sample, evsel, al, addr_al); > + python_process_tracepoint(sample, evsel, al, addr_al, machine); > break; > /* Reserve for future process_hw/sw/raw APIs */ > default: > if (tables->db_export_mode) > db_export__sample(&tables->dbe, event, sample, evsel, al, addr_al); > else > - python_process_general_event(sample, evsel, al, addr_al); > + python_process_general_event(sample, evsel, al, addr_al, machine); > } > } > > diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c > index 7172ca052..089a2c905 100644 > --- a/tools/perf/util/trace-event-scripting.c > +++ b/tools/perf/util/trace-event-scripting.c > @@ -51,7 +51,8 @@ static void process_event_unsupported(union perf_event *event __maybe_unused, > struct perf_sample *sample __maybe_unused, > struct evsel *evsel __maybe_unused, > struct addr_location *al __maybe_unused, > - struct addr_location *addr_al __maybe_unused) > + struct addr_location *addr_al __maybe_unused, > + struct machine *machine __maybe_unused) > { > } > > diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h > index 640981105..6f5b1a6d5 100644 > --- a/tools/perf/util/trace-event.h > +++ b/tools/perf/util/trace-event.h > @@ -81,7 +81,8 @@ struct scripting_ops { > struct perf_sample *sample, > struct evsel *evsel, > struct addr_location *al, > - struct addr_location *addr_al); > + struct addr_location *addr_al, > + struct machine *machine); > void (*process_switch)(union perf_event *event, > struct perf_sample *sample, > struct machine *machine); > -- > 2.25.1 > >
Hi Athira, On 02/12/2021 16:03, Athira Rajeev wrote: > >> On 01-Dec-2021, at 6:03 PM, German Gomez <german.gomez@arm.com> wrote: >> >> Include a "struct machine*" parameter to the process_event callback in >> the scripting layer. This will allow access to the perf_env from within >> this callback. >> >> Followup patches will build on top of this to report the correct name of >> the registers in a perf.data file, consistently with the architecture >> the file was recorded in. >> >> Signed-off-by: German Gomez <german.gomez@arm.com> >> --- >> tools/perf/builtin-script.c | 2 +- >> .../util/scripting-engines/trace-event-perl.c | 3 ++- >> .../scripting-engines/trace-event-python.c | 23 +++++++++++-------- >> tools/perf/util/trace-event-scripting.c | 3 ++- >> tools/perf/util/trace-event.h | 3 ++- >> 5 files changed, 21 insertions(+), 13 deletions(-) >> >> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c >> index 9434367af..711132f0b 100644 >> --- a/tools/perf/builtin-script.c >> +++ b/tools/perf/builtin-script.c >> @@ -2256,7 +2256,7 @@ static int process_sample_event(struct perf_tool *tool, >> thread__resolve(al.thread, &addr_al, sample); >> addr_al_ptr = &addr_al; >> } >> - scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr); >> + scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr, machine); > Hi, > > Looks like the patch is using “machine” to allow access to perf_env__arch and there by to get the “arch” value. > But can we use from evsel, like "perf_env__arch(evsel__env(evsel))” to get arch value instead of including new parameter for “struct machine” ? > > Thanks > Athira Thanks for the suggestion. It looks like we can skip this patch if we can get the arch value that way. Thanks! German
Em Fri, Dec 03, 2021 at 12:00:26PM +0000, German Gomez escreveu: > Hi Athira, > > On 02/12/2021 16:03, Athira Rajeev wrote: > > > >> On 01-Dec-2021, at 6:03 PM, German Gomez <german.gomez@arm.com> wrote: > >> > >> Include a "struct machine*" parameter to the process_event callback in > >> the scripting layer. This will allow access to the perf_env from within > >> this callback. > >> > >> Followup patches will build on top of this to report the correct name of > >> the registers in a perf.data file, consistently with the architecture > >> the file was recorded in. > >> > >> Signed-off-by: German Gomez <german.gomez@arm.com> > >> --- > >> tools/perf/builtin-script.c | 2 +- > >> .../util/scripting-engines/trace-event-perl.c | 3 ++- > >> .../scripting-engines/trace-event-python.c | 23 +++++++++++-------- > >> tools/perf/util/trace-event-scripting.c | 3 ++- > >> tools/perf/util/trace-event.h | 3 ++- > >> 5 files changed, 21 insertions(+), 13 deletions(-) > >> > >> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > >> index 9434367af..711132f0b 100644 > >> --- a/tools/perf/builtin-script.c > >> +++ b/tools/perf/builtin-script.c > >> @@ -2256,7 +2256,7 @@ static int process_sample_event(struct perf_tool *tool, > >> thread__resolve(al.thread, &addr_al, sample); > >> addr_al_ptr = &addr_al; > >> } > >> - scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr); > >> + scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr, machine); > > Hi, > > > > Looks like the patch is using “machine” to allow access to perf_env__arch and there by to get the “arch” value. > > But can we use from evsel, like "perf_env__arch(evsel__env(evsel))” to get arch value instead of including new parameter for “struct machine” ? > > > > Thanks > > Athira > > Thanks for the suggestion. It looks like we can skip this patch if we > can get the arch value that way. So the first one is already upstream, will you post a v2? - Arnaldo
On 13/12/2021 18:22, Arnaldo Carvalho de Melo wrote: > [...] > >> Thanks for the suggestion. It looks like we can skip this patch if we >> can get the arch value that way. > So the first one is already upstream, will you post a v2? > > - Arnaldo Hi Arnaldo, I sent a v2 where this patch was removed. The upstreamed patch (1/4 here) remained the same. https://lore.kernel.org/all/20211207180653.1147374-1-german.gomez@arm.com/
On December 13, 2021 3:31:47 PM GMT-03:00, German Gomez <german.gomez@arm.com> wrote: > >On 13/12/2021 18:22, Arnaldo Carvalho de Melo wrote: >> [...] >> >>> Thanks for the suggestion. It looks like we can skip this patch if we >>> can get the arch value that way. >> So the first one is already upstream, will you post a v2? >> >> - Arnaldo > >Hi Arnaldo, > >I sent a v2 where this patch was removed. The upstreamed patch (1/4 >here) remained the same. Thanks, I'll check. - Arnaldo > >https://lore.kernel.org/all/20211207180653.1147374-1-german.gomez@arm.com/
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 9434367af..711132f0b 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -2256,7 +2256,7 @@ static int process_sample_event(struct perf_tool *tool, thread__resolve(al.thread, &addr_al, sample); addr_al_ptr = &addr_al; } - scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr); + scripting_ops->process_event(event, sample, evsel, &al, addr_al_ptr, machine); } else { process_event(scr, sample, evsel, &al, &addr_al, machine); } diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c index 32a721b3e..6017c4660 100644 --- a/tools/perf/util/scripting-engines/trace-event-perl.c +++ b/tools/perf/util/scripting-engines/trace-event-perl.c @@ -454,7 +454,8 @@ static void perl_process_event(union perf_event *event, struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, - struct addr_location *addr_al) + struct addr_location *addr_al, + struct machine *machine __maybe_unused) { scripting_context__update(scripting_context, event, sample, evsel, al, addr_al); perl_process_tracepoint(sample, evsel, al); diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c index c0c010350..e164f8d00 100644 --- a/tools/perf/util/scripting-engines/trace-event-python.c +++ b/tools/perf/util/scripting-engines/trace-event-python.c @@ -708,7 +708,8 @@ static void regs_map(struct regs_dump *regs, uint64_t mask, char *bf, int size) static void set_regs_in_dict(PyObject *dict, struct perf_sample *sample, - struct evsel *evsel) + struct evsel *evsel, + struct machine *machine __maybe_unused) { struct perf_event_attr *attr = &evsel->core.attr; @@ -776,6 +777,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, struct addr_location *addr_al, + struct machine *machine, PyObject *callchain) { PyObject *dict, *dict_sample, *brstack, *brstacksym; @@ -849,7 +851,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, PyLong_FromUnsignedLongLong(sample->cyc_cnt)); } - set_regs_in_dict(dict, sample, evsel); + set_regs_in_dict(dict, sample, evsel, machine); return dict; } @@ -857,7 +859,8 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, static void python_process_tracepoint(struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, - struct addr_location *addr_al) + struct addr_location *addr_al, + struct machine *machine) { struct tep_event *event = evsel->tp_format; PyObject *handler, *context, *t, *obj = NULL, *callchain; @@ -964,7 +967,7 @@ static void python_process_tracepoint(struct perf_sample *sample, PyTuple_SetItem(t, n++, dict); if (get_argument_count(handler) == (int) n + 1) { - all_entries_dict = get_perf_sample_dict(sample, evsel, al, addr_al, + all_entries_dict = get_perf_sample_dict(sample, evsel, al, addr_al, machine, callchain); PyTuple_SetItem(t, n++, all_entries_dict); } else { @@ -1366,7 +1369,8 @@ static int python_process_call_return(struct call_return *cr, u64 *parent_db_id, static void python_process_general_event(struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, - struct addr_location *addr_al) + struct addr_location *addr_al, + struct machine *machine) { PyObject *handler, *t, *dict, *callchain; static char handler_name[64]; @@ -1388,7 +1392,7 @@ static void python_process_general_event(struct perf_sample *sample, /* ip unwinding */ callchain = python_process_callchain(sample, evsel, al); - dict = get_perf_sample_dict(sample, evsel, al, addr_al, callchain); + dict = get_perf_sample_dict(sample, evsel, al, addr_al, machine, callchain); PyTuple_SetItem(t, n++, dict); if (_PyTuple_Resize(&t, n) == -1) @@ -1403,7 +1407,8 @@ static void python_process_event(union perf_event *event, struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, - struct addr_location *addr_al) + struct addr_location *addr_al, + struct machine *machine) { struct tables *tables = &tables_global; @@ -1411,14 +1416,14 @@ static void python_process_event(union perf_event *event, switch (evsel->core.attr.type) { case PERF_TYPE_TRACEPOINT: - python_process_tracepoint(sample, evsel, al, addr_al); + python_process_tracepoint(sample, evsel, al, addr_al, machine); break; /* Reserve for future process_hw/sw/raw APIs */ default: if (tables->db_export_mode) db_export__sample(&tables->dbe, event, sample, evsel, al, addr_al); else - python_process_general_event(sample, evsel, al, addr_al); + python_process_general_event(sample, evsel, al, addr_al, machine); } } diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c index 7172ca052..089a2c905 100644 --- a/tools/perf/util/trace-event-scripting.c +++ b/tools/perf/util/trace-event-scripting.c @@ -51,7 +51,8 @@ static void process_event_unsupported(union perf_event *event __maybe_unused, struct perf_sample *sample __maybe_unused, struct evsel *evsel __maybe_unused, struct addr_location *al __maybe_unused, - struct addr_location *addr_al __maybe_unused) + struct addr_location *addr_al __maybe_unused, + struct machine *machine __maybe_unused) { } diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h index 640981105..6f5b1a6d5 100644 --- a/tools/perf/util/trace-event.h +++ b/tools/perf/util/trace-event.h @@ -81,7 +81,8 @@ struct scripting_ops { struct perf_sample *sample, struct evsel *evsel, struct addr_location *al, - struct addr_location *addr_al); + struct addr_location *addr_al, + struct machine *machine); void (*process_switch)(union perf_event *event, struct perf_sample *sample, struct machine *machine);
Include a "struct machine*" parameter to the process_event callback in the scripting layer. This will allow access to the perf_env from within this callback. Followup patches will build on top of this to report the correct name of the registers in a perf.data file, consistently with the architecture the file was recorded in. Signed-off-by: German Gomez <german.gomez@arm.com> --- tools/perf/builtin-script.c | 2 +- .../util/scripting-engines/trace-event-perl.c | 3 ++- .../scripting-engines/trace-event-python.c | 23 +++++++++++-------- tools/perf/util/trace-event-scripting.c | 3 ++- tools/perf/util/trace-event.h | 3 ++- 5 files changed, 21 insertions(+), 13 deletions(-)