Re: [PATCH v2 3/3] coresight: tmc: Decouple the perf buffer allocation from sysfs mode
From: Leo Yan <leo.yan@arm.com>
Date: 2025-07-02 17:08:30
Also in:
lkml
On Fri, Jun 20, 2025 at 03:54:12PM +0800, Junhao He wrote: [..]
quoted hunk ↗ jump to hunk
@@ -1341,33 +1339,24 @@ alloc_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event, unsigned long size; node = (event->cpu == -1) ? NUMA_NO_NODE : cpu_to_node(event->cpu); - /* - * Try to match the perf ring buffer size if it is larger - * than the size requested via sysfs. - */ - if ((nr_pages << PAGE_SHIFT) > drvdata->size) { - etr_buf = tmc_alloc_etr_buf(drvdata, ((ssize_t)nr_pages << PAGE_SHIFT), - 0, node, NULL); - if (!IS_ERR(etr_buf)) - goto done; - } + + /* Use the minimum limit if the required size is smaller */ + size = (unsigned long)nr_pages << PAGE_SHIFT;
Please change the size's type to ssize_t, then:
size = nr_pages << PAGE_SHIFT;
+ if (size < TMC_ETR_PERF_MIN_BUF_SIZE) + size = TMC_ETR_PERF_MIN_BUF_SIZE;
size = min_t(ssize_t, size, TMC_ETR_PERF_MIN_BUF_SIZE);
/*
- * 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;
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.
Thanks,
Leo
return ERR_PTR(-ENOMEM); - -done: - return etr_buf; } static struct etr_buf * -- 2.33.0