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

[Leo Yan <leo.yan@arm.com>]

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)