On Tue, Mar 11, 2025 at 10:51:39AM +0000, James Clark wrote:
[...]
+Sink selection +~~~~~~~~~~~~~~
+An appropriate sink will be selected automatically for use with Perf, but since +there will typically be more than one sink, the name of the sink to use may be +specified as a special config option prefixed with '@'.
+The available sinks are listed in sysFS under ($SYSFS)/bus/event_source/devices/cs_etm/sinks/::
root@localhost:/sys/bus/event_source/devices/cs_etm/sinks# ls tmc_etf0 tmc_etr0 tpiu0Just a minor comment. To reflect the latest hardware, it is good to mention the TRBE case, users should not and cannot specify TRBE as the
Is that strictly true? From looking at the code I think you could pick one TRBE sink as long as you are only tracing from a single ETM. Although yeah it would be pointless.
My testing result shows perf reports error when specifying trbe as sink:
# perf record -e cs_etm/@trbe0/ -- ls failed to mmap with 12 (Cannot allocate memory)
But I can make success for the command:
# perf record -C 0 -e cs_etm/@trbe0/ -- ls [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.062 MB perf.data ]
This makes sense to me that if a perf session tries to enable trace for multiple CPUs but only a CPU sink is supported, the driver should report error to remind users the command does not work.
sink name. The driver will give priority for TRBE by default unless users specify other sink types.
IMO mentioning TRBE would be overly wordy and not really add anything. Removing the sink from all the base examples is exactly to make TRBE work without going into detail about why. And the advanced section doesn't mention TRBE because manually picking it is never right.
The section "Sink selection" applies _only_ to traditional sinks, not to TRBE. I am not sure if we should clarify a bit for this. I don't have strong opinion on it, as it is a trade-off between providing necessary info and avoiding overstatement.
So this patch is fine for me:
Reviewed-by: Leo Yan leo.yan@arm.com