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

[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@xxxxxxxxxxx>  
> > > 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@xxxxxxxxxxx>
> > > > > > > > ---
> > > > > > > >    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@xxxxxxxxxxxxxxxx
> > > To unsubscribe send an email tocoresight-leave@xxxxxxxxxxxxxxxx