Re: [PATCH] coresight: trbe: Use scope-based resource management in arm_trbe_alloc_buffer()

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.

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;

    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)