On 2025/7/3 1:08, Leo Yan wrote:
/*
* Else switch to configured size for this ETR* and scale down until we hit the minimum limit.
* Try to allocate the required size for this ETR, if failed scale* down until we hit the minimum limit.
- size = drvdata->size; do { etr_buf = tmc_alloc_etr_buf(drvdata, size, 0, node, NULL); if (!IS_ERR(etr_buf))
goto done;
return etr_buf;Do we really need to scale down buffer size for failure cases? I would like a straightforward code:
etr_buf = tmc_alloc_etr_buf(drvdata, size, 0, node, NULL); if (IS_ERR_OR_NULL(etr_buf)) return etr_buf;
I conducted test on my platform, and the logs are as follows:
[root@localhost tmc_etr0]# cat buf_modes_available auto flat [root@localhost etm]# perf record -e cs_etm// -C 0 -m,8M -- stress -c 1 -t 10 failed to mmap with 22 (Invalid argument) [root@localhost etm]# perf record -e cs_etm// -C 0 -m,4M -- stress -c 1 -t 10 stress: info: [1727601] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd stress: info: [1727601] successful run completed in 10s [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 9.358 MB perf.data ] [root@localhost etm]# perf record -e cs_etm// -C 0 -- stress -c 1 -t 10 stress: info: [1727717] dispatching hogs: 1 cpu, 0 io, 0 vm, 0 hdd stress: info: [1727717] successful run completed in 10s [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 9.380 MB perf.data ]
If the user-specified mmap pages exceed the maximum number of pages supported by ETR, the perf process will return a failure. If the user has a method to obtain the maximum number of pages supported by ETR, I believe that scaling down the buffer size for failure cases is unnecessary.
Best regards, Junhao.
Just a side topic, we know tmc_alloc_etr_buf() should not return NULL pointer. For a sanity check, the callers (alloc_etr_buf(), tmc_etr_get_sysfs_buffer(), etc) should valid a buffer pointer with IS_ERR_OR_NULL() rather than IS_ERR(). This can be a separate patch.