Re: [PATCH v7 07/14] xen/arm: add support for cache coloring configuration via device-tree

[Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>]

Hi Jan,

On Tue, Mar 19, 2024 at 4:41 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 15.03.2024 11:58, Carlo Nonato wrote:
> > --- a/xen/common/llc-coloring.c
> > +++ b/xen/common/llc-coloring.c
> > @@ -253,6 +253,37 @@ int domain_set_llc_colors(struct domain *d,
> >      return 0;
> >  }
> >
> > +int __init domain_set_llc_colors_from_str(struct domain *d, const char 
> > *str)
> > +{
> > +    int err;
> > +    unsigned int *colors, num_colors;
> > +
> > +    if ( !str )
> > +        return domain_set_default_colors(d);
> > +
> > +    colors = xmalloc_array(unsigned int, max_nr_colors);
> > +    if ( !colors )
> > +        return -ENOMEM;
> > +
> > +    err = parse_color_config(str, colors, max_nr_colors, &num_colors);
> > +    if ( err )
> > +    {
> > +        printk(XENLOG_ERR "Error parsing LLC color configuration");
> > +        return err;
> > +    }
> > +
> > +    if ( !check_colors(colors, num_colors) )
> > +    {
> > +        printk(XENLOG_ERR "Bad LLC color config for %pd\n", d);
> > +        return -EINVAL;
> > +    }
>
> "colors" is again leaked on the error paths.

Yep.

> > +    d->llc_colors = colors;
> > +    d->num_llc_colors = num_colors;
>
> num_colors may be quite a bit smaller than max_nr_colors; worth re-
> allocating the array to free up excess space?

Don't know if it's worth it, but it's a very small change so I think I can add
it.

> Jan

Thanks.