Hi Greg,
On 3/25/2022 1:06 AM, Greg Kroah-Hartman wrote:
On Thu, Mar 24, 2022 at 10:23:19PM +0800, 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.
How many "more"? Where does the trade off of speed for complexity help? How much faster is this really? You can't claim performance improvements without any proof :)
There is about 40 trace sources in our internal device. I believe there will be more cpu cores, then there will be more etm sources. IDR here is used to store the path of both etm sources and other sources which aren't associated with CPU. Use IDR is not more complicated than using list. It will also save the time of searching the path when coresight_disable. I tested in internal device. The test case is that enable all the sources, disable the source one by one to check the search time.
Use list to store paths:
sh-7687 [005] .... 342.113099: __coresight_disable: ====search path start==== source_0 sh-7687 [005] .... 342.113127: __coresight_disable: ====search path end==== source_0 sh-7693 [005] .... 342.542216: __coresight_disable: ====search path start==== source_1 sh-7693 [005] .... 342.542244: __coresight_disable: ====search path end==== source_1 sh-7699 [005] .... 342.929083: __coresight_disable: ====search path start==== source_2 sh-7699 [005] .... 342.929106: __coresight_disable: ====search path end==== source_2 sh-7711 [005] .... 343.760688: __coresight_disable: ====search path start==== source_3 sh-7711 [005] .... 343.760713: __coresight_disable: ====search path end==== source_3 sh-7717 [005] .... 344.172353: __coresight_disable: ====search path start==== source_4 sh-7717 [005] .... 344.172381: __coresight_disable: ====search path end==== source_4
Use IDR to store paths:
sh-7156 [006] .... 223.294228: __coresight_disable: ====search path start==== source_0 sh-7156 [006] .... 223.294237: __coresight_disable: ====search path end==== source_0 sh-7162 [006] .... 223.690153: __coresight_disable: ====search path start==== source_1 sh-7162 [006] .... 223.690163: __coresight_disable: ====search path end==== source_1 sh-7168 [006] .... 224.110670: __coresight_disable: ====search path start==== source_2 sh-7168 [006] .... 224.110679: __coresight_disable: ====search path end==== source_2 <...>-7180 [006] .... 224.929315: __coresight_disable: ====search path start==== source_3 <...>-7180 [006] .... 224.929324: __coresight_disable: ====search path end==== source_3 <...>-7186 [006] .... 225.343617: __coresight_disable: ====search path start==== source_4 <...>-7186 [006] .... 225.343626: __coresight_disable: ====search path end==== source_4
From the log, Searching the path from the IDR takes about 9us for each source. Searching the path from the list takes about 23 ~ 28us for the source. Use IDR saves much time.
Thanks
Jinlong Mao
thanks,
greg k-h