Re: [PATCH 08/19] coresight: trbe: Use PERF_AUX_FLAG_PARTIAL instead of PERF_AUX_FLAG_COLLISION

[James Clark <james.clark@linaro.org>]

On 05/12/2025 4:28 am, Anshuman Khandual wrote:
> On 01/12/25 4:51 PM, Leo Yan wrote:
>> When tracing stops, there is no collision with other samples, so using
>> PERF_AUX_FLAG_COLLISION does not accurately reflect the trace state and
>> may mislead userspace.
>>
>> Use PERF_AUX_FLAG_PARTIAL instead to indicate that tracing stopped and
>> the record may contain gaps.
>>
>> Signed-off-by: Leo Yan <leo.yan@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-trbe.c | 19 ++++++-------------
>>   1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
>> index b06885a08e082fd34f68d9588518807b5c47c86e..0caa4a6b437a3aa39fc6bcc72a23711b54f7c598 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -669,21 +669,14 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
>>   	}
>>   
>>   	/*
>> -	 * Mark the buffer to indicate that there was a WRAP event by
>> -	 * setting the COLLISION flag. This indicates to the user that
>> -	 * the TRBE trace collection was stopped without stopping the
>> -	 * ETE and thus there might be some amount of trace that was
>> -	 * lost between the time the WRAP was detected and the IRQ
>> -	 * was consumed by the CPU.
>> -	 *
>> -	 * Setting the TRUNCATED flag would move the event to STOPPED
>> -	 * state unnecessarily, even when there is space left in the
>> -	 * ring buffer. Using the COLLISION flag doesn't have this side
>> -	 * effect. We only set TRUNCATED flag when there is no space
>> -	 * left in the ring buffer.
>> +	 * Mark the buffer to indicate that the trace is stopped by setting
>> +	 * the PARTIAL flag. This indicates to the user that the TRBE trace
>> +	 * collection was stopped without stopping the ETE and thus there
>> +	 * might be some amount of trace that was lost between the time the
>> +	 * TRBE event was detected and the IRQ was consumed by the CPU.
>>   	 */
>>   	if (!is_trbe_running(trbsr))
>> -		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
>> +		perf_aux_output_flag(handle, PERF_AUX_FLAG_PARTIAL);
> 
> This is interesting. So there is a no possibility for the records to be
> overridden in the perf ring buffer and incomplete record is all that can
> happen when TRBE is stopped, without stopping the ETE first ? Could both
> of these scenarios might be possible as well ?
> 

There's no chance of overwriting because that would be the TRUNCATED 
case and the event would get disabled. PARTIAL seems to be when there 
was an error that's recoverable in the driver only, but as in my other 
comment I don't think the extra complexity is necessary and always using 
TRUNCATED would be enough.

>>   
>>   	if (is_trbe_wrap(trbsr))
>>   		return TRBE_FAULT_ACT_WRAP;
>>