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 tpiu0 > > > > Just 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@xxxxxxx>
