Re: [PATCH] coresight: trbe: Use scope-based resource management in arm_trbe_alloc_buffer()
From: Leo Yan <leo.yan@arm.com>
Date: 2025-09-12 10:27:45
Also in:
lkml
On Fri, Sep 12, 2025 at 09:55:57AM +0530, Anshuman Khandual wrote: [...]
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.
Though include/linux/cleanup.h suggests to "always define and assign variables in one statement" to avoid potential interdependency problem with lock, this is not concerned in arm_trbe_alloc_buffer(). So I bias to Anshuman's suggestion of declaring variables at the top of the function, as this is the style widely used in the kernel.
quoted hunk ↗ jump to hunk
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 trbe_buf *buf __free(kfree) = NULL;
+ struct page **pglist __free(kfree);
struct page **pglist __free(kfree) = NULL;
quoted hunk ↗ jump to hunk
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)