[PATCH V7 4/4] Adjust objdump start/end range per map pgoff parameter
Steve Clevenger
scclevenger at os.amperecomputing.com
Fri Sep 6 10:27:16 PDT 2024
On 9/6/2024 4:27 AM, Leo Yan wrote:
>
>
> On 9/5/24 23:28, Steve Clevenger wrote:
>>
>> Extract map_pgoff parameter from the dictionary, and adjust start/end
>> range passed to objdump based on the value.
>>
>> A zero start_addr is filtered to prevent output of dso address range
>> check failures. This script repeatedly sees a zero value passed
>> in for
>> start_addr = cpu_data[str(cpu) + 'addr']
>>
>> These zero values are not a new problem. The start_addr/stop_addr warning
>> clutters the instruction trace output, hence this change.
>>
>> Signed-off-by: Steve Clevenger <scclevenger at os.amperecomputing.com>
>> ---
>> tools/perf/scripts/python/arm-cs-trace-disasm.py | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/
>> perf/scripts/python/arm-cs-trace-disasm.py
>> index 7aff02d84ffb..a867e0db02b8 100755
>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> @@ -187,6 +187,7 @@ def process_event(param_dict):
>> dso_start = get_optional(param_dict, "dso_map_start")
>> dso_end = get_optional(param_dict, "dso_map_end")
>> symbol = get_optional(param_dict, "symbol")
>> + map_pgoff = get_optional(param_dict, "map_pgoff")
>
>
> I am concerned the two sentences below are inconsistence: one uses
> 'start_addr + map_pgoff' and the other uses 'start_addr + int(map_pgoff)'.
>
Valid point. It's working fine as is, but how is it even working? I look
at print_disam/read_disasm, and no type conversion is done in either
call. The dso_start parameter for these calls is actually dso_vm_start
which is the dso_start integer conversion.
> Here about below code?
>
> map_pgoff_str = get_optional(param_dict, "map_pgoff")
> if map_pgoff_str == "":
> map_pgoff = 0
> else:
> map_pgoff = int(map_pgoff_str)
>
> With above change, 'map_pgoff' is an int type. As a result, the changes
> below can simply add 'map_pgoff'.
I propose:
map_pgoff_str = get_optional(param_dict, "map_pgoff"
if (map_pgoff_str.isdigit()):
map_pgoff = int(map_pgoff)
else:
map_pgoff = 0
The int() conversions in the print() statement can then be removed.
Steve
> With these changes, LGTM:
>
> Reviewed-by: Leo Yan <leo.yan at arm.com>
>
>
>> cpu = sample["cpu"]
>> ip = sample["ip"]
>> @@ -243,9 +244,11 @@ def process_event(param_dict):
>> # Record for previous sample packet
>> cpu_data[str(cpu) + 'addr'] = addr
>>
>> - # Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4
>> - if (start_addr == 0 and stop_addr == 4):
>> - print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
>> + # Filter out zero start_address. Optionally identify
>> CS_ETM_TRACE_ON packet
>> + # if start_addr=0 and stop_addr=4.
>> + if (start_addr == 0):
>> + if ((stop_addr == 4) and (options.verbose == True)):
>> + print("CPU%d: CS_ETM_TRACE_ON packet is
>> inserted" % cpu)
>> return
>>
>> if (start_addr < int(dso_start) or start_addr > int(dso_end)):
>> @@ -262,13 +265,14 @@ def process_event(param_dict):
>> # vm_start to zero.
>> if (dso == "[kernel.kallsyms]" or dso_start ==
>> 0x400000):
>> dso_vm_start = 0
>> + map_pgoff = 0
>> else:
>> dso_vm_start = int(dso_start)
>>
>> dso_fname = get_dso_file_path(dso, dso_bid)
>> if path.exists(dso_fname):
>> - print_disam(dso_fname, dso_vm_start,
>> start_addr, stop_addr)
>> + print_disam(dso_fname, dso_vm_start,
>> start_addr + map_pgoff, stop_addr + map_pgoff)
>> else:
>> - print("Failed to find dso %s for address range
>> [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
>> + print("Failed to find dso %s for address range
>> [ 0x%x .. 0x%x ]" % (dso, start_addr + int(map_pgoff), stop_addr +
>> int(map_pgoff)))
>
>>
>> print_srccode(comm, param_dict, sample, symbol, dso)
>> --
>> 2.44.0
>>
More information about the linux-arm-kernel
mailing list