Re: [PATCH] coresight: trbe: Use scope-based resource management in arm_trbe_alloc_buffer()
From: Anshuman Khandual <hidden>
Date: 2025-09-12 04:26:03
Also in:
lkml
Subsystem:
arm/coresight framework and drivers, hardware tracing facilities, the rest · Maintainers:
Suzuki K Poulose, Alexander Shishkin, Linus Torvalds
On 09/09/25 5:58 PM, Markus Elfring wrote:
From: Markus Elfring <redacted>
Date: Tue, 9 Sep 2025 14:20:06 +0200
Scope-based resource management became supported for some
programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SHA ID here just needs first 12 digits not the entire string.
Introduce __cleanup() based infrastructure"). * Thus use the attribute “__free(kfree)”. * Reduce the scopes for the local variables “buf” and “pglist”.
What is that required ?
* Omit four kfree() calls accordingly.
Right. The commit message should be re-written with little more context from arm_trbe_alloc_buffer(), after describing the new scope-based resource management.
quoted hunk ↗ jump to hunk
Signed-off-by: Markus Elfring <redacted> --- drivers/hwtracing/coresight/coresight-trbe.c | 21 ++++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-)diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 8f9bbef71f23..1b0d58bf8613 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c@@ -733,8 +733,6 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, struct perf_event *event, void **pages, int nr_pages, bool snapshot) { - struct trbe_buf *buf; - struct page **pglist; int i; /*@@ -746,32 +744,29 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, if (nr_pages < 2) return NULL; - buf = kzalloc_node(sizeof(*buf), GFP_KERNEL, trbe_alloc_node(event)); + struct trbe_buf *buf __free(kfree) = kzalloc_node(sizeof(*buf), + GFP_KERNEL, + trbe_alloc_node(event)); if (!buf) return NULL; - pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); - if (!pglist) { - kfree(buf); + struct page **pglist __free(kfree) = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); + if (!pglist) return NULL; - } for (i = 0; i < nr_pages; i++) pglist[i] = virt_to_page(pages[i]); buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); - if (!buf->trbe_base) { - kfree(pglist); - kfree(buf); + if (!buf->trbe_base) return NULL; - } + buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; buf->snapshot = snapshot; buf->nr_pages = nr_pages; buf->pages = pages; - kfree(pglist); - return buf; + return_ptr(buf); } static void arm_trbe_free_buffer(void *config)
Seems like a good idea though. But please keep the local variable declaration in the current place which is bit cleaner and reduces code churn as well.
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
index 7a226316c48f..7babba1a9afb 100644
--- a/drivers/hwtracing/coresight/coresight-trbe.c
+++ b/drivers/hwtracing/coresight/coresight-trbe.c@@ -733,8 +733,8 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, struct perf_event *event, void **pages, int nr_pages, bool snapshot) { - struct trbe_buf *buf; - struct page **pglist; + struct trbe_buf *buf __free(kfree); + struct page **pglist __free(kfree); int i; /*
@@ -751,27 +751,22 @@ static void *arm_trbe_alloc_buffer(struct coresight_device *csdev, return NULL; pglist = kcalloc(nr_pages, sizeof(*pglist), GFP_KERNEL); - if (!pglist) { - kfree(buf); + if (!pglist) return NULL; - } for (i = 0; i < nr_pages; i++) pglist[i] = virt_to_page(pages[i]); buf->trbe_base = (unsigned long)vmap(pglist, nr_pages, VM_MAP, PAGE_KERNEL); - if (!buf->trbe_base) { - kfree(pglist); - kfree(buf); + if (!buf->trbe_base) return NULL; - } + buf->trbe_limit = buf->trbe_base + nr_pages * PAGE_SIZE; buf->trbe_write = buf->trbe_base; buf->snapshot = snapshot; buf->nr_pages = nr_pages; buf->pages = pages; - kfree(pglist); - return buf; + return_ptr(buf); } static void arm_trbe_free_buffer(void *config)