mbox series

[V5,0/4] arm-cs-trace-disasm.py/perf must accommodate non-zero DSO text offset

Message ID cover.1724879699.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

Message

Steve Clevenger Aug. 28, 2024, 11:17 p.m. UTC
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 (4):
  Add dso__is_pie call to identify ELF PIE
  Force MAPPING_TYPE__IDENTIY for PIE
  Add map pgoff to python dictionary based on MAPPING_TYPE
  Adjust objdump start/end range per map pgoff parameter

 .../scripts/python/arm-cs-trace-disasm.py     |  9 ++-
 tools/perf/util/map.c                         |  4 +-
 .../scripting-engines/trace-event-python.c    | 13 +++-
 tools/perf/util/symbol-elf.c                  | 61 +++++++++++++++++++
 tools/perf/util/symbol.h                      |  1 +
 5 files changed, 80 insertions(+), 8 deletions(-)

Comments

Leo Yan Aug. 29, 2024, 7:55 a.m. UTC | #1
On 8/29/2024 12:17 AM, Steve Clevenger wrote:
> 
> 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.

For the series:

Reviewed-by: Leo Yan <leo.yan@arm.com>

Hi Steve,

Later when you respin a new version patches, it is better to amend the review
and ACK tags you have received. (Just remind, b4 is your friend for helping do
these things).

Thanks,
Leo
Steve Clevenger Aug. 29, 2024, 5:39 p.m. UTC | #2
On 8/29/2024 12:55 AM, Leo Yan wrote:
> On 8/29/2024 12:17 AM, Steve Clevenger wrote:
>>
>> 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.
> 
> For the series:
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
> Hi Steve,
> 
> Later when you respin a new version patches, it is better to amend the review
> and ACK tags you have received. (Just remind, b4 is your friend for helping do
> these things).
> 
> Thanks,
> Leo

Got it. Thanks, Leo. By the way, I noticed when I rebased to
perf-tools-next the kernel version was 6.11-rc3. Is there any chance
this patch gets into 6.11?

Steve
Leo Yan Aug. 30, 2024, 7:20 a.m. UTC | #3
On 8/29/24 18:39, Steve Clevenger wrote:> 
> On 8/29/2024 12:55 AM, Leo Yan wrote:
>> On 8/29/2024 12:17 AM, Steve Clevenger wrote:
>>>
>>> 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.
>>
>> For the series:
>>
>> Reviewed-by: Leo Yan <leo.yan@arm.com>
>>
>> Hi Steve,
>>
>> Later when you respin a new version patches, it is better to amend the review
>> and ACK tags you have received. (Just remind, b4 is your friend for helping do
>> these things).
>>
>> Thanks,
>> Leo
> 
> Got it. Thanks, Leo. By the way, I noticed when I rebased to
> perf-tools-next the kernel version was 6.11-rc3. Is there any chance
> this patch gets into 6.11?

Linus' master branch now is 6.11-rc5, it is not far away from the 6.11.

I think we still have much chance to get it done before it. Added maintainers
in case any missing.

Thanks,
Leo
Steve Clevenger Aug. 31, 2024, 7:19 p.m. UTC | #4
On 8/30/2024 12:20 AM, Leo Yan wrote:
> On 8/29/24 18:39, Steve Clevenger wrote:>
>> On 8/29/2024 12:55 AM, Leo Yan wrote:
>>> On 8/29/2024 12:17 AM, Steve Clevenger wrote:
>>>>
>>>> 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.
>>>
>>> For the series:
>>>
>>> Reviewed-by: Leo Yan <leo.yan@arm.com>
>>>
>>> Hi Steve,
>>>
>>> Later when you respin a new version patches, it is better to amend
>>> the review
>>> and ACK tags you have received. (Just remind, b4 is your friend for
>>> helping do
>>> these things).
>>>
>>> Thanks,
>>> Leo
>>
>> Got it. Thanks, Leo. By the way, I noticed when I rebased to
>> perf-tools-next the kernel version was 6.11-rc3. Is there any chance
>> this patch gets into 6.11?
> 
> Linus' master branch now is 6.11-rc5, it is not far away from the 6.11.
> 
> I think we still have much chance to get it done before it. Added
> maintainers
> in case any missing.
> 
> Thanks,
> Leo

Hi Leo,

That's great. Unfortuantely, I need to submit a V6 for patch 4/4, which
is the arm-cs-trace-disasm.py script. Kernel instruction trace requires
zeroing map_pgoff. perf makes most offset decisions, but this one is in
the script. It's a one liner.

I know you're a b4 fan, but I've never used it. If I'm able to make
progress with it I'll use it to resubmit. Otherwise more of the same.

Steve

Steve
Leo Yan Sept. 1, 2024, 1:53 p.m. UTC | #5
Hi Steve,

On 8/31/2024 8:19 PM, Steve Clevenger wrote:

[...]

> Hi Leo,
> 
> That's great. Unfortuantely, I need to submit a V6 for patch 4/4, which
> is the arm-cs-trace-disasm.py script. Kernel instruction trace requires
> zeroing map_pgoff. perf makes most offset decisions, but this one is in
> the script. It's a one liner.

Agreed the kernel symbols should be zero map offset. 

Thanks for correcting this.

> I know you're a b4 fan, but I've never used it. If I'm able to make
> progress with it I'll use it to resubmit. Otherwise more of the same.

For using the b4 command:

$ b4 am <Message-ID>

You can find the message ID in the lore webpage, e.g. for this series,
in the cover letter link [1], click the 'raw' hyper link, then you can
fine the Message ID is:
cover.1724879699.git.scclevenger@os.amperecomputing.com

Run 'b4 am cover.1724879699.git.scclevenger@os.amperecomputing.com' for
downloading the patches (with automatic amending ACK and review tags).

As result, you will get the file:
v5_20240828_scclevenger_arm_cs_trace_disasm_py_perf_must_accommodate_non_zero_dso_text_offset.mbx

$ git am v5_20240828_scclevenger_arm_cs_trace_disasm_py_perf_must_accommodate_non_zero_dso_text_offset.mbx

Alternatively, if you are using new b4 version, you can directly use
the command 'b4 shazam <Message-ID>' for downloading series and applying
it on your local branch in one go.

I tried the b4 command for your series but failed, I can see you used `git
send-email` but the series was not discontinuous so b4 command cannot fetch
the whole series.

You can just work on you existed patches - don't forget to append my review
tags for patches 01 ~ 03. For the patch 04, after you send out I will review
it.

Thanks,
Leo    
  
[1] https://lore.kernel.org/linux-perf-users/7a2c53c0-0bac-4a53-824d-6efdff8ed3ce@os.amperecomputing.com/T/#mc76180d8076cf3414ba81e7c379cda93297aa4a9