[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6920de94-a9c8-47f4-840f-391d1ec85c0c@os.amperecomputing.com>
Date: Mon, 22 Jul 2024 15:32:00 +0530
From: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
To: James Clark <james.clark@...aro.org>, james.clark@....com,
mike.leach@...aro.org, suzuki.poulose@....com, Leo Yan <leo.yan@....com>
Cc: acme@...hat.com, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
darren@...amperecomputing.com, scclevenger@...amperecomputing.com
Subject: Re: [PATCH] perf scripts python arm-cs-trace-disasm.py: Skip disasm
if address continuity is broken
Hi James,
On 19-07-2024 08:09 pm, James Clark wrote:
>
>
> On 19/07/2024 10:26 am, Ganapatrao Kulkarni wrote:
>> To generate the instruction tracing, script uses 2 contiguous packets
>> address range. If there a continuity brake due to discontiguous branch
>> address, it is required to reset the tracing and start tracing with the
>> new set of contiguous packets.
>>
>> Adding change to identify the break and complete the remaining tracing
>> of current packets and restart tracing from new set of packets, if
>> continuity is established.
>>
>
> Hi Ganapatrao,
>
> Can you add a before and after example of what's changed to the commit
> message? It wasn't immediately obvious to me if this is adding missing
> output, or it was correcting the tail end of the output that was
> previously wrong.
It is adding tail end of the trace as well avoiding the segfault of the
perf application. With out this change the perf segfaults with as below log
./perf script --script=python:./scripts/python/arm-cs-trace-disasm.py --
-d objdump -k ../../vmlinux -v $* > dump
objdump: error: the stop address should be after the start address
Traceback (most recent call last):
File "./scripts/python/arm-cs-trace-disasm.py", line 271, in
process_event
print_disam(dso_fname, dso_vm_start, start_addr, stop_addr)
File "./scripts/python/arm-cs-trace-disasm.py", line 105, in print_disam
for line in read_disam(dso_fname, dso_start, start_addr, stop_addr):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "./scripts/python/arm-cs-trace-disasm.py", line 99, in read_disam
disasm_output = check_output(disasm).decode('utf-8').split('\n')
^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/subprocess.py", line 466, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/subprocess.py", line 571, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['objdump', '-d', '-z',
'--start-address=0xffff80008125b758',
'--stop-address=0xffff80008125a934', '../../vmlinux']' returned non-zero
exit status 1.
Fatal Python error: handler_call_die: problem in Python trace event handler
Python runtime state: initialized
Current thread 0x0000ffffb05054e0 (most recent call first):
<no Python frame>
Extension modules: perf_trace_context, systemd._journal,
systemd._reader, systemd.id128, report._py3report, _dbus_bindings,
problem._py3abrt (total: 7)
Aborted (core dumped)
>
>> Signed-off-by: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
>> ---
>> tools/perf/scripts/python/arm-cs-trace-disasm.py | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> index d973c2baed1c..ad10cee2c35e 100755
>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> @@ -198,6 +198,10 @@ def process_event(param_dict):
>> cpu_data[str(cpu) + 'addr'] = addr
>> return
>> + if (cpu_data.get(str(cpu) + 'ip') == None):
>> + cpu_data[str(cpu) + 'ip'] = ip
>> +
>
> Do you need to write into the global cpu_data here? Doesn't it get
> overwritten after you load it back into 'prev_ip'
No, the logic is same as holding the addr of previous packet.
Saving the previous packet saved ip in to prev_ip before overwriting
with the current packet.
>
> prev_ip = cpu_data[str(cpu) + 'ip']
>
> ... then ...
>
> # Record for previous sample packet
> cpu_data[str(cpu) + 'addr'] = addr
> cpu_data[str(cpu) + 'ip'] = stop_addr
>
> Would a local variable not accomplish the same thing?
No, We need global to hold the ip of previous packet.
>
>> + prev_ip = cpu_data[str(cpu) + 'ip']
>> if (options.verbose == True):
>> print("Event type: %s" % name)
>> @@ -243,12 +247,18 @@ def process_event(param_dict):
>> # Record for previous sample packet
>> cpu_data[str(cpu) + 'addr'] = addr
>> + cpu_data[str(cpu) + 'ip'] = stop_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)
>> return
>> + if (stop_addr < start_addr):
>> + # Continuity of the Packets broken, set start_addr to previous
>> + # packet ip to complete the remaining tracing of the address
>> range.
>> + start_addr = prev_ip
>> +
>> if (start_addr < int(dso_start) or start_addr > int(dso_end)):
>> print("Start address 0x%x is out of range [ 0x%x .. 0x%x ]
>> for dso %s" % (start_addr, int(dso_start), int(dso_end), dso))
>> return
Thanks,
Ganapat
Powered by blists - more mailing lists