Re: [PATCH 07/19] coresight: trbe: Refactor AUX flag setting

[Leo Yan <leo.yan@arm.com>]

On Tue, Dec 02, 2025 at 11:15:47AM +0000, Suzuki K Poulose wrote:

[...]

> > @@ -687,6 +668,23 @@ static enum trbe_fault_action trbe_get_fault_act(struct perf_output_handle *hand
> >   		goto out_fatal;
> >   	}
> > +	/*
> > +	 * 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.
> > +	 */
> > +	if (!is_trbe_running(trbsr))
> 
> Is this really required ? There is a WARN_ON(is_trbe_running(trbsr))
> above ?

The WARN_ON() will reports warning for running case, here it sets
COLLISION flag for not running case, which is not conflict.

This works for the Fill mode (stop collection), we always set the
COLLISION flag for it.  Once we enable trigger and wrap modes, due to
the trace continues to run, we don't set the flag for these modes.

> > +		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
> 
> Also, setting this would unnecessarily mark COLLISION while stopping the
> TRBE buffer normally (when called from arm_trbe_update_buffer), when
> there is no WRAP event ?

Good point.  I should refactor arm_trbe_update_buffer(), it firstly
reads out TRBSR, stop trace afterwards, and then call
trbe_get_fault_act().

In this case, it is in running mode so COLLISION will not be set for
normal disable flow.

I will add a new patch for this and place it before this one.

> > +
> >   	if (is_trbe_wrap(trbsr))
> >   		return TRBE_FAULT_ACT_WRAP;
> 
> Couldn't this be :
> 
> 	/* Move the big fat comment here */
> 	if (is_trbe_wrap(trbsr)) {
> 		perf_aux_output_flag(handle, PERF_AUX_FLAG_COLLISION);
> 		return TRBE_FAULT_ACT_WRAP;
> 	}

It deliberately de-couples the COLLISION flag from the WRAP event.

In later change, a WRAP event only means buffer wrap around, it does
mean the trace has been stopped.  The AUX flag is only set for
stopping case.

Thanks,
Leo