Re: [PATCH v2] cxl/acpi: Cleanup __cxl_parse_cfmws()

[Dan Williams <dan.j.williams@intel.com>]

Alison Schofield wrote:
> On Wed, Feb 28, 2024 at 12:30:56PM -0800, Dan Williams wrote:
> > As a follow on to the recent rework of __cxl_parse_cfmws() to always
> > return errors [1], use cleanup.h helpers to remove goto and other cleanups
> > now that logging is moved to the cxl_parse_cfmws() wrapper.
> 
> one question about helper below...
> 
> 
> >
> snip
> 
> > +static struct resource *alloc_cxl_resource(resource_size_t base,
> > +					   resource_size_t n, const char *fmt,
> > +					   ...)
> > +{
> > +	va_list ap;
> > +
> > +	struct resource *res __free(kfree) = kzalloc(sizeof(*res), GFP_KERNEL);
> > +	if (!res)
> > +		return NULL;
> > +
> > +	res->start = base;
> > +	res->end = base + n - 1;
> > +	res->flags = IORESOURCE_MEM;
> > +
> > +	va_start(ap, fmt);
> > +	res->name = kvasprintf(GFP_KERNEL, fmt, ap);
> > +	va_end(ap);
> 
> Why is alloc_cxl_resource() defined w variable params and using
> kvasprintf(), rather than tailored to the one caller and used
> kasprintf()?
> 
> Perhaps you have bigger plans for this helper?

Good point, thanks! I think this function can just take the window id for now.

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 1a47207932e2..db38ead0353a 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -325,25 +325,20 @@ static void del_cxl_resource(struct resource *res)
 }
 
 static struct resource *alloc_cxl_resource(resource_size_t base,
-                                          resource_size_t n, const char *fmt,
-                                          ...)
+                                          resource_size_t n, int id)
 {
-       va_list ap;
-
        struct resource *res __free(kfree) = kzalloc(sizeof(*res), GFP_KERNEL);
+
        if (!res)
                return NULL;
 
        res->start = base;
        res->end = base + n - 1;
        res->flags = IORESOURCE_MEM;
-
-       va_start(ap, fmt);
-       res->name = kvasprintf(GFP_KERNEL, fmt, ap);
-       va_end(ap);
-
+       res->name = kasprintf(GFP_KERNEL, "CXL Window %d", id);
        if (!res->name)
                return NULL;
+
        return no_free_ptr(res);
 }
 
@@ -384,9 +379,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
        for (i = 0; i < ways; i++)
                target_map[i] = cfmws->interleave_targets[i];
 
-       struct resource *res __free(del_cxl_resource) =
-               alloc_cxl_resource(cfmws->base_hpa, cfmws->window_size,
-                                  "CXL Window %d", ctx->id++);
+       struct resource *res __free(del_cxl_resource) = alloc_cxl_resource(
+               cfmws->base_hpa, cfmws->window_size, ctx->id++);
        if (!res)
                return -ENOMEM;