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;
 		size /= 2;
 	} while (size >= TMC_ETR_PERF_MIN_BUF_SIZE);

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.