Messages in this thread |  | | Date | Mon, 11 Apr 2022 10:55:50 +0800 | Subject | Re: [PATCH v4 01/10] Use IDR to maintain all the enabled sources' paths. | From | Jinlong Mao <> |
| |
On 3/30/2022 5:05 PM, Suzuki K Poulose wrote: > 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@quicinc.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@quicinc.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
Hi Suzuki,
I will address your comments in next version.
Could you please help to review other patches ?
Thanks
Jinlong Mao
> >> >> Thanks >> Jinlong Mao >>>> Thanks >>>> >>>> Jinlong Mao >>>> >>>>> Cheers >>>>> Suzuki >>>>> >>>>> >>>>>>> thanks, >>>>>>> >>>>>>> greg k-h >>>>>> Thanks >>>>>> >>>>>> Jinlong Mao >>>>>> >>> _______________________________________________ >>> CoreSight mailing list --coresight@lists.linaro.org >>> To unsubscribe send an email tocoresight-leave@lists.linaro.org >
|  |