[PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

Suzuki K Poulose suzuki.poulose at arm.com
Fri Oct 13 03:07:37 AEDT 2023


Hi,

On 12/10/2023 16:56, Athira Rajeev wrote:
> 
> 
>> On 05-Oct-2023, at 3:06 PM, Suzuki K Poulose <suzuki.poulose at arm.com> wrote:
>>
>> On 05/10/2023 06:02, Namhyung Kim wrote:
>>> On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
>>> <atrajeev at linux.vnet.ibm.com> wrote:
>>>>

...

>> Thanks for the fix.
>>
>> Nothing to do with this patch, but I am wondering if the original patch
>> is over engineered and may not be future proof.
>>
>> e.g.,
>>
>> cs_etm_dev_name() {
>> + cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit)
>>
>> Right there you got the device name and we can easily deduce the name of
>> the "ETM" node.
>>
>> e.g,:
>> etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")
>>
>> And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
>> system (hopefully, no one builds it ;-))
>>
>> Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
>> we should simply use the cpu nodes from :
>>
>> /sys/bus/event_source/devices/cs_etm/
>>
>> e.g.,
>>
>> arm_cs_etm_traverse_path_test() {
>> # Iterate for every ETM device
>> for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
>> # Read the link to be on the safer side
>> dev=`readlink $c`
>>
>> # Find the ETM device belonging to which CPU
>> cpu=`cat $dev/cpu`
>>
>> # Use depth-first search (DFS) to iterate outputs
>> arm_cs_iterate_devices $dev $cpu
>> done;
>> }
>>
>>
>>
>>> You'd better add Coresight folks on this.
>>> Maybe this file was missing in the MAINTAINERS file.
>>
>> And the original author of the commit, that introduced the issue too.
>>
>> Suzuki
> 
> Hi All,
> Thanks for the discussion and feedbacks.
> 
> This patch fixes the shellcheck warning introduced in function "cs_etm_dev_name". But with the changes that Suzuki suggested, we won't need the function "cs_etm_dev_name" since the code will use "/sys/bus/event_source/devices/cs_etm/" .  In that case, can I drop this patch for now from this series ?
> 

Yes, please. James will send out the proposed patch

Suzuki




More information about the Linuxppc-dev mailing list