[PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh
Athira Rajeev
atrajeev at linux.vnet.ibm.com
Fri Oct 13 03:20:27 AEDT 2023
> On 12-Oct-2023, at 9:37 PM, Suzuki K Poulose <suzuki.poulose at arm.com> wrote:
>
> 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
Hi Suzuki,
Sure. Thanks!
Athira
>
> Suzuki
>
>
More information about the Linuxppc-dev
mailing list