1. Patchew
  2. linux
  3. [PATCH v2 0/3] Coresight TMC-ETR some bugfixes and cleanups
  4. View patch

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

Junhao He posted 3 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 3/3] coresight: tmc: Decouple the perf buffer allocation from sysfs mode
Posted by Junhao He 3 months, 2 weeks ago 
From: Yicong Yang <yangyicong@hisilicon.com>

Currently the perf buffer allocation follows the below logic:
- if the required AUX buffer size if larger, allocate the buffer with
  the required size
- otherwise allocate the size reference to the sysfs buffer size

This is not useful as we only collect to one AUX data, so just try to
allocate the buffer match the AUX buffer size.

Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Link: https://lore.kernel.org/linux-arm-kernel/df8967cd-2157-46a2-97d9-a1aea883cf63@arm.com/
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Signed-off-by: Junhao He <hejunhao3@huawei.com>
---
 .../hwtracing/coresight/coresight-tmc-etr.c   | 29 ++++++-------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 252a57a8e94e..94dc968a76da 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1327,9 +1327,7 @@ EXPORT_SYMBOL_GPL(tmc_etr_get_buffer);
 
 /*
  * alloc_etr_buf: Allocate ETR buffer for use by perf.
- * The size of the hardware buffer is dependent on the size configured
- * via sysfs and the perf ring buffer size. We prefer to allocate the
- * largest possible size, scaling down the size by half until it
+ * Allocate the largest possible size, scaling down the size by half until it
  * reaches a minimum limit (1M), beyond which we give up.
  */
 static struct etr_buf *
@@ -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;
+	if (size < TMC_ETR_PERF_MIN_BUF_SIZE)
+		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);
 
 	return ERR_PTR(-ENOMEM);
-
-done:
-	return etr_buf;
 }
 
 static struct etr_buf *
-- 
2.33.0
Re: [PATCH v2 3/3] coresight: tmc: Decouple the perf buffer allocation from sysfs mode
Posted by Leo Yan 3 months, 1 week ago 
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
>
Re: [PATCH v2 3/3] coresight: tmc: Decouple the perf buffer allocation from sysfs mode
Posted by hejunhao 2 months, 3 weeks ago 
On 2025/7/3 1:08, Leo Yan wrote:
> 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);

Sure, I will do that.

>>   
>>   	/*
>> -	 * 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.

A new patch will be added to achieve this.

Thank you for your comments.
Junhao.

> Thanks,
> Leo
>
>>   	return ERR_PTR(-ENOMEM);
>> -
>> -done:
>> -	return etr_buf;
>>   }
>>   
>>   static struct etr_buf *
>> -- 
>> 2.33.0
>>
> .
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.