[PATCH v2 13/16] coresight: tmc-etr: Allow events to use the same ETR buffer

Suzuki K Poulose suzuki.poulose at arm.com
Wed Mar 27 04:32:14 PDT 2019


On 03/26/2019 05:55 PM, Mathieu Poirier wrote:
> On Tue, Mar 26, 2019 at 04:18:35PM +0000, Suzuki K Poulose wrote:
>> On 03/25/2019 09:56 PM, Mathieu Poirier wrote:
>>> This patch uses the pid of the process being traced to aggregate traces
>>> coming from different processors in the same sink, something that is
>>> required when collecting traces in CPU-wide mode when the CoreSight HW
>>> enacts a N:1 source/sink topology.
>>>
>>> Signed-off-by: Mathieu Poirier <mathieu.poirier at linaro.org>
>>> ---
>>>    .../hwtracing/coresight/coresight-tmc-etr.c   | 71 +++++++++++++++++--
>>>    1 file changed, 65 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> index 7254fafdf1c2..cbabf88bd51d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>>> @@ -8,6 +8,7 @@
>>>    #include <linux/coresight.h>
>>>    #include <linux/dma-mapping.h>
>>>    #include <linux/iommu.h>
>>> +#include <linux/idr.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/types.h>
>>>    #include <linux/vmalloc.h>
>>> @@ -41,6 +42,9 @@ struct etr_perf_buffer {
>>>    	void			**pages;
>>>    };
>>> +static DEFINE_IDR(session_idr);
>>> +static DEFINE_MUTEX(session_idr_lock);
>>
>> Please correct me if I am wrong here. What we now do with this series is
>>
>> - One event per CPU and thus one ETR perf buf per CPU.
>> - One *ETR buf* per PID, from the IDR hash list.
>>
>> However, if we have 1:1 situation, we will have :
>>
>> N (say 2 ETRs), but one *ETR buf* as they all share the same PID, implying
>> multiple multiple ETRs will try to use the same etr buf,
>> which could corrupt the trace data ? Instead,  what we need in that
>> situation is :
>>
>> One ETR buf perf PID+ETR device. i.e, etr_bufs must be per ETR.
>> So should the IDR be specific to an ETR ?
>>
>> Or do we not support a session with multiple ETRs involved (1:1) ?
> 
> At this time 1:1 topologies are not supported and a fair amount of work will go
> in doing so.  When I started working on this serie my thought was that because
> of said amount of work there is no point thinking about 1:1, and that we can
> deal with it when we get there.
> 
> But taking a step back and having dealt with the harder (concurrency) problems
> inherent to CPU-wide scenarios, it would be trivial to make the code 1:1 ready
> and it will be one less thing to worry about down the road.
> 
> That being said and answering your question above, I think we need and IDR per
> ETR (in the drvdata) to avoid contention issues on systems with several ETRs.
> 
> Thanks for bringing this back to my attention.

Cool. Thanks for explaining the rationale. So, when we do that, I think
we may be able to have one "etr_perf_buffer" per ETR and thus we may be
able to move the refcount back to the etr_perf_buffer, just like we
moved the PID and index etr_perf_buffer instead of the etr_buf ?

Cheers
Suzuki




More information about the linux-arm-kernel mailing list