linux-kernel - Re: [PATCH v4 01/10] Use IDR to maintain all the enabled sources' paths.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <ff1931d7-61b4-4362-7e3b-45c64c0211fc@arm.com>
Date:   Wed, 30 Mar 2022 10:05:00 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Jinlong Mao <quic_jinlmao@...cinc.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Mike Leach <mike.leach@...aro.org>, coresight@...ts.linaro.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Tingwei Zhang <quic_tingweiz@...cinc.com>,
        Yuanfang Zhang <quic_yuanfang@...cinc.com>,
        Tao Zhang <quic_taozha@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Hao Zhang <quic_hazha@...cinc.com>,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v4 01/10] Use IDR to maintain all the enabled sources'
 paths.

On 30/03/2022 03:10, Jinlong Mao wrote:
> 
> On 3/29/2022 10:36 PM, Mathieu Poirier wrote:
>> On Tue, 29 Mar 2022 at 07:56, Jinlong Mao<quic_jinlmao@...cinc.com>  wrote:
>>> Hi Suzuki,
>>>
>>> On 3/28/2022 4:33 PM, Suzuki K Poulose wrote:
>>>> On 24/03/2022 14:23, Jinlong Mao wrote:
>>>>> Hi Greg,
>>>>>
>>>>> Thanks for your review.
>>>>>
>>>>> On 3/24/2022 8:26 PM, Greg Kroah-Hartman wrote:
>>>>>> On Thu, Mar 24, 2022 at 08:17:25PM +0800, Mao Jinlong wrote:
>>>>>>> Use hash length of the source's device name to map to the pointer
>>>>>>> of the enabled path. Using IDR will be more efficient than using
>>>>>>> the list. And there could be other sources except STM and CPU etms
>>>>>>> in the new HWs. It is better to maintain all the paths together.
>>>>>>>
>>>>>>> Signed-off-by: Mao Jinlong<quic_jinlmao@...cinc.com>
>>>>>>> ---
>>>>>>>    drivers/hwtracing/coresight/coresight-core.c | 75
>>>>>>> +++++++-------------
>>>>>>>    1 file changed, 26 insertions(+), 49 deletions(-)
>>>>>> Your subject line is odd.  Please put back the driver subsystem in the
>>>>>> subject line so that it makes more sense.
>>>>> I will update the subject in next version.
>>>>>> And how have you measured "more efficient"?
>>>>> Using IDR would be better than doing a sequential search as there
>>>>> will be much more device  in future.
>>>> Where do we use sequential search now ? For non-CPU bound sources, yes
>>>> we may need something. But CPU case is straight forward, and could be
>>>> retained as it is. i.e., per-cpu list of paths.
>>>>
>>> We use list to store the paths for both ETM and non-CPU bound sources in
>>> patch below.
>>>
>>> “[PATCH 01/10] coresight: add support to enable more coresight paths”
>>>
>>> According to Mathieu's comments, IDR is used now.  So i added "Using IDR
>>> will be more efficient than using
>>> the list" this message in my commit message. I think we need to use one
>>> mechanism to store ETM and
>>> non-CPU bound sources.
>>>
>>>
>>> Mathieu's comments:
>>>
>>> So many TPDM and many ETMs...  That is definitely a reason to do better than a
>>> sequential search.
>>>
>>> If an IDR (or some other kind of mechanism) is used then we can use that to
>>> store paths associated with ETMs as well.  That way everything works the same
>>> way and access time is constant for any kind of source.
>> As per my last sentence above, the goal of  my comment was to simplify
>> things so that we don't have two different ways of managing sources.
>> But if that ends up causing more trouble than benefit then it should
>> be avoided.
> 
> Hi Mathieu,
> 
> I didn't see any disadvantage to use IDR to store both ETM source and 
> non-CPU bound sources.
> 
> Benefits:
> 
>   * Only need to maintain one way of managing sources.
>   * Less time to search the path

My preference is to keep the ETM source paths per-CPU. For the reasons
below :
   - It is straight forward for an ETM. per_cpu(paths, cpu)
   - It is faster than the IDR.
   - Makes the debugging easier. Simply lookup the per_cpu variable.

I agree that the IDR is required for the non ETM sources. And I am fine
with that.

Suzuki

> 
> Thanks
> Jinlong Mao
>>> Thanks
>>>
>>> Jinlong Mao
>>>
>>>> Cheers
>>>> Suzuki
>>>>
>>>>
>>>>>> thanks,
>>>>>>
>>>>>> greg k-h
>>>>> Thanks
>>>>>
>>>>> Jinlong Mao
>>>>>
>> _______________________________________________
>> CoreSight mailing list --coresight@...ts.linaro.org
>> To unsubscribe send an email tocoresight-leave@...ts.linaro.org