Re: [PATCH v2 3/3] coresight: tmc: Decouple the perf buffer allocation from sysfs mode

On Fri, Jun 20, 2025 at 03:54:12PM +0800, Junhao He wrote:

[..]

@@ -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