| Message ID | cover.1728599785.git.scclevenger@os.amperecomputing.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset | expand |
Hello, On Fri, Oct 11, 2024 at 11:17:10AM -0600, Steve Clevenger wrote: > Changes in V9: > - Removed V8 patch files 1/4 and 2/4. > - Modified set_sym_in_dict (trace-event-python.c) to add map_pgoff > in dictionary as-is without regard to MAPPING_IDENTITY. This patch > file is now patch 2/2. I think the previous version had Leo's Reviewed-by tag. Leo, can you confirm if it still holds? Thanks, Namhyung > > Changes in V8: > - in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to > string. > - Remove map_pgoff integer conversion in dso not found print > message. > > Changes in V7: > - In arm-cs-trace-disasm.py, fix print message core dump resulting > from mixed type arithmetic. > - Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The > CS_ETM_TRACE_ON message is changed to print only in verbose mode. > - Removed verbose mode only notification for start_addr/stop_addr > outside of dso address range. > > Changes in V6: > - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add > map_pgoff to start/end address for dso not found message. > - Added "Reviewed-by" trailer for patches 1-3 previously reviewed > by Leo Yan in V4 and V5. > > Changes in V5: > - In symbol-elf.c, branch to exit_close label if open file. > - In trace_event_python.c, correct indentation. set_sym_in_dict > call parameter "map_pgoff" renamed as "addr_map_pgoff" to > match local naming. > > Changes in V4: > - In trace-event-python.c, fixed perf-tools-next merge problem. > > Changes in V3: > - Rebased to linux-perf-tools branch. > - Squash symbol-elf.c and symbol.h into same commit. > - In map.c, merge dso__is_pie() call into existing if statement. > - In arm-cs-trace-disasm.py, remove debug artifacts. > > Changes in V2: > - In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer > checks per Leo Yan review. > - Updated mailing list distribution. > > Steve Clevenger (2): > Add map_pgoff to python dictionary > Adjust objdump start/end address per map_pgoff parameter > > tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++----- > .../util/scripting-engines/trace-event-python.c | 9 ++++++--- > 2 files changed, 17 insertions(+), 8 deletions(-) > > -- > 2.44.0 >
On 10/16/2024 10:51 AM, Namhyung Kim wrote: > Hello, > > On Fri, Oct 11, 2024 at 11:17:10AM -0600, Steve Clevenger wrote: >> Changes in V9: >> - Removed V8 patch files 1/4 and 2/4. >> - Modified set_sym_in_dict (trace-event-python.c) to add map_pgoff >> in dictionary as-is without regard to MAPPING_IDENTITY. This patch >> file is now patch 2/2. > > I think the previous version had Leo's Reviewed-by tag. > > Leo, can you confirm if it still holds? > > Thanks, > Namhyung It did. You can confirm there's no arm-cs-trace-python.py script change from V8. Note the patch file numbering is different (since 2 patches dropped). The trace-event-python.c patch file changed so I had to drop out his "Reviewed-by" tag for that file. Due to the patch numbering change, I didn't want to add confusion so I left it out. Steve > >> >> Changes in V8: >> - in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to >> string. >> - Remove map_pgoff integer conversion in dso not found print >> message. >> >> Changes in V7: >> - In arm-cs-trace-disasm.py, fix print message core dump resulting >> from mixed type arithmetic. >> - Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The >> CS_ETM_TRACE_ON message is changed to print only in verbose mode. >> - Removed verbose mode only notification for start_addr/stop_addr >> outside of dso address range. >> >> Changes in V6: >> - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add >> map_pgoff to start/end address for dso not found message. >> - Added "Reviewed-by" trailer for patches 1-3 previously reviewed >> by Leo Yan in V4 and V5. >> >> Changes in V5: >> - In symbol-elf.c, branch to exit_close label if open file. >> - In trace_event_python.c, correct indentation. set_sym_in_dict >> call parameter "map_pgoff" renamed as "addr_map_pgoff" to >> match local naming. >> >> Changes in V4: >> - In trace-event-python.c, fixed perf-tools-next merge problem. >> >> Changes in V3: >> - Rebased to linux-perf-tools branch. >> - Squash symbol-elf.c and symbol.h into same commit. >> - In map.c, merge dso__is_pie() call into existing if statement. >> - In arm-cs-trace-disasm.py, remove debug artifacts. >> >> Changes in V2: >> - In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer >> checks per Leo Yan review. >> - Updated mailing list distribution. >> >> Steve Clevenger (2): >> Add map_pgoff to python dictionary >> Adjust objdump start/end address per map_pgoff parameter >> >> tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++----- >> .../util/scripting-engines/trace-event-python.c | 9 ++++++--- >> 2 files changed, 17 insertions(+), 8 deletions(-) >> >> -- >> 2.44.0 >>
Hi Steve, On Wed, Nov 06, 2024 at 04:51:11PM -0700, Steve Clevenger wrote: [...] > Changes in V10: > - Removed errant space in patch file 0002. Passed 'git apply --check' > at perf-tools-next, 6.11.0-rc6. > - Added back missing prefixes. The subject prefixes in two patches are not quite right. The subjects would be: perf script python: Add map_pgoff to python dictionary perf script cs-etm: Adjust objdump start/end range per map pgoff parameter With above change, you could keep my review tags. Please use git commands to generate patches ('git commit --amend' and 'git format-patch'). If directly modify the plain patch files and resend them, I suspect this is the reason that the archieve website messes up rendering the patch series v9 and v10 [1]. Thanks, Leo [1] https://lore.kernel.org/linux-perf-users/cover.1728599785.git.scclevenger@os.amperecomputing.com/T/#t
On Wed, Nov 06, 2024 at 04:51:10PM -0700, Steve Clevenger wrote: > Add map_pgoff parameter to python dictionary so it can be seen by the > python script. > > Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> > Reviewed-by: Leo Yan <leo.yan@arm.com> > --- > tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c > index d7183134b669..367132b3a51a 100644 > --- a/tools/perf/util/scripting-engines/trace-event-python.c > +++ b/tools/perf/util/scripting-engines/trace-event-python.c > @@ -793,7 +793,8 @@ static int set_regs_in_dict(PyObject *dict, > static void set_sym_in_dict(PyObject *dict, struct addr_location *al, > const char *dso_field, const char *dso_bid_field, > const char *dso_map_start, const char *dso_map_end, > - const char *sym_field, const char *symoff_field) > + const char *sym_field, const char *symoff_field, > + const char *map_pgoff) > { > char sbuild_id[SBUILD_ID_SIZE]; > > @@ -809,6 +810,8 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al, > PyLong_FromUnsignedLong(map__start(al->map))); > pydict_set_item_string_decref(dict, dso_map_end, > PyLong_FromUnsignedLong(map__end(al->map))); > + pydict_set_item_string_decref(dict, map_pgoff, > + PyLong_FromUnsignedLongLong(al->map->pgoff)); Please use map__pgoff(al->map) instead. Otherwise you'll get this in the debug build: util/scripting-engines/trace-event-python.c: In function 'set_sym_in_dict': util/scripting-engines/trace-event-python.c:814:60: error: 'struct map' has no member named 'pgoff' 814 | PyLong_FromUnsignedLongLong(al->map->pgoff)); | ^~ Thanks, Namhyung > } > if (al->sym) { > pydict_set_item_string_decref(dict, sym_field, > @@ -895,7 +898,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, > pydict_set_item_string_decref(dict, "comm", > _PyUnicode_FromString(thread__comm_str(al->thread))); > set_sym_in_dict(dict, al, "dso", "dso_bid", "dso_map_start", "dso_map_end", > - "symbol", "symoff"); > + "symbol", "symoff", "map_pgoff"); > > pydict_set_item_string_decref(dict, "callchain", callchain); > > @@ -920,7 +923,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, > PyBool_FromLong(1)); > set_sym_in_dict(dict_sample, addr_al, "addr_dso", "addr_dso_bid", > "addr_dso_map_start", "addr_dso_map_end", > - "addr_symbol", "addr_symoff"); > + "addr_symbol", "addr_symoff", "addr_map_pgoff"); > } > > if (sample->flags) > -- > 2.44.0 >
On 11/7/2024 10:51 AM, Namhyung Kim wrote: > On Wed, Nov 06, 2024 at 04:51:10PM -0700, Steve Clevenger wrote: >> Add map_pgoff parameter to python dictionary so it can be seen by the >> python script. >> >> Signed-off-by: Steve Clevenger <scclevenger@os.amperecomputing.com> >> Reviewed-by: Leo Yan <leo.yan@arm.com> >> --- >> tools/perf/util/scripting-engines/trace-event-python.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c >> index d7183134b669..367132b3a51a 100644 >> --- a/tools/perf/util/scripting-engines/trace-event-python.c >> +++ b/tools/perf/util/scripting-engines/trace-event-python.c >> @@ -793,7 +793,8 @@ static int set_regs_in_dict(PyObject *dict, >> static void set_sym_in_dict(PyObject *dict, struct addr_location *al, >> const char *dso_field, const char *dso_bid_field, >> const char *dso_map_start, const char *dso_map_end, >> - const char *sym_field, const char *symoff_field) >> + const char *sym_field, const char *symoff_field, >> + const char *map_pgoff) >> { >> char sbuild_id[SBUILD_ID_SIZE]; >> >> @@ -809,6 +810,8 @@ static void set_sym_in_dict(PyObject *dict, struct addr_location *al, >> PyLong_FromUnsignedLong(map__start(al->map))); >> pydict_set_item_string_decref(dict, dso_map_end, >> PyLong_FromUnsignedLong(map__end(al->map))); >> + pydict_set_item_string_decref(dict, map_pgoff, >> + PyLong_FromUnsignedLongLong(al->map->pgoff)); > > Please use map__pgoff(al->map) instead. Otherwise you'll get this in > the debug build: > > util/scripting-engines/trace-event-python.c: In function 'set_sym_in_dict': > util/scripting-engines/trace-event-python.c:814:60: error: 'struct map' has no member named 'pgoff' > 814 | PyLong_FromUnsignedLongLong(al->map->pgoff)); > | ^~ > > Thanks, > Namhyung > Hi Namhyung, I do not see this compile error, and I typically build perf with debug. This make command works through 6.11.0-rc6: $ make -C tools/perf DEBUG=1 VF=1 CORESIGHT=1 PYTHON=python3 CONFIG_LIBPYTHON=y CSLIBS=/usr/lib CSINCLUDES=/usr/include install I can substitute in the map__pgoff() macro in any case. Thanks, Steve > >> } >> if (al->sym) { >> pydict_set_item_string_decref(dict, sym_field, >> @@ -895,7 +898,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, >> pydict_set_item_string_decref(dict, "comm", >> _PyUnicode_FromString(thread__comm_str(al->thread))); >> set_sym_in_dict(dict, al, "dso", "dso_bid", "dso_map_start", "dso_map_end", >> - "symbol", "symoff"); >> + "symbol", "symoff", "map_pgoff"); >> >> pydict_set_item_string_decref(dict, "callchain", callchain); >> >> @@ -920,7 +923,7 @@ static PyObject *get_perf_sample_dict(struct perf_sample *sample, >> PyBool_FromLong(1)); >> set_sym_in_dict(dict_sample, addr_al, "addr_dso", "addr_dso_bid", >> "addr_dso_map_start", "addr_dso_map_end", >> - "addr_symbol", "addr_symoff"); >> + "addr_symbol", "addr_symoff", "addr_map_pgoff"); >> } >> >> if (sample->flags) >> -- >> 2.44.0 >>
On Fri, Nov 8, 2024 at 9:58 AM Steve Clevenger <scclevenger@os.amperecomputing.com> wrote: > On 11/7/2024 10:51 AM, Namhyung Kim wrote: > > Please use map__pgoff(al->map) instead. Otherwise you'll get this in > > the debug build: > > > > util/scripting-engines/trace-event-python.c: In function 'set_sym_in_dict': > > util/scripting-engines/trace-event-python.c:814:60: error: 'struct map' has no member named 'pgoff' > > 814 | PyLong_FromUnsignedLongLong(al->map->pgoff)); > > | ^~ > > > > Thanks, > > Namhyung > > > > Hi Namhyung, > > I do not see this compile error, and I typically build perf with debug. > This make command works through 6.11.0-rc6: > > $ make -C tools/perf DEBUG=1 VF=1 CORESIGHT=1 PYTHON=python3 > CONFIG_LIBPYTHON=y CSLIBS=/usr/lib CSINCLUDES=/usr/include install > > I can substitute in the map__pgoff() macro in any case. The map_pgoff was added for reference count checking which changes struct layout when enabled. More context here: https://perfwiki.github.io/main/reference-count-checking/ Thanks, Ian
Changes in V9: - Removed V8 patch files 1/4 and 2/4. - Modified set_sym_in_dict (trace-event-python.c) to add map_pgoff in dictionary as-is without regard to MAPPING_IDENTITY. This patch file is now patch 2/2. Changes in V8: - in arm-cs-trace-disasm.py, ensure map_pgoff is not converted to string. - Remove map_pgoff integer conversion in dso not found print message. Changes in V7: - In arm-cs-trace-disasm.py, fix print message core dump resulting from mixed type arithmetic. - Modify CS_ETM_TRACE_ON filter to filter zero start_addr. The CS_ETM_TRACE_ON message is changed to print only in verbose mode. - Removed verbose mode only notification for start_addr/stop_addr outside of dso address range. Changes in V6: - In arm-cs-trace-disasm.py, zero map_pgoff for kernel files. Add map_pgoff to start/end address for dso not found message. - Added "Reviewed-by" trailer for patches 1-3 previously reviewed by Leo Yan in V4 and V5. Changes in V5: - In symbol-elf.c, branch to exit_close label if open file. - In trace_event_python.c, correct indentation. set_sym_in_dict call parameter "map_pgoff" renamed as "addr_map_pgoff" to match local naming. Changes in V4: - In trace-event-python.c, fixed perf-tools-next merge problem. Changes in V3: - Rebased to linux-perf-tools branch. - Squash symbol-elf.c and symbol.h into same commit. - In map.c, merge dso__is_pie() call into existing if statement. - In arm-cs-trace-disasm.py, remove debug artifacts. Changes in V2: - In dso__is_pie() (symbol-elf.c), Decrease indentation, add null pointer checks per Leo Yan review. - Updated mailing list distribution. Steve Clevenger (2): Add map_pgoff to python dictionary Adjust objdump start/end address per map_pgoff parameter tools/perf/scripts/python/arm-cs-trace-disasm.py | 16 +++++++++++----- .../util/scripting-engines/trace-event-python.c | 9 ++++++--- 2 files changed, 17 insertions(+), 8 deletions(-)