Re: [PATCH v9 09/13] xen: add cache coloring allocator for domains

[Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>]

Hi Jan

On Tue, Nov 5, 2024 at 5:11 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 25.10.2024 11:50, Carlo Nonato wrote:
> > --- a/docs/misc/xen-command-line.pandoc
> > +++ b/docs/misc/xen-command-line.pandoc
> > @@ -270,6 +270,20 @@ and not running softirqs. Reduce this if softirqs are 
> > not being run frequently
> >  enough. Setting this to a high value may cause boot failure, particularly 
> > if
> >  the NMI watchdog is also enabled.
> >
> > +### buddy-alloc-size (arm64)
> > +> `= <size>`
> > +
> > +> Default: `64M`
> > +
> > +Amount of memory reserved for the buddy allocator when colored allocator is
> > +active. This options is available only when LLC coloring support is 
> > enabled.
> > +The colored allocator is meant as an alternative to the buddy allocator,
> > +because its allocation policy is by definition incompatible with the 
> > generic
> > +one. Since the Xen heap systems is not colored yet, we need to support the
> > +coexistence of the two allocators for now. This parameter, which is 
> > optional
> > +and for expert only, it's used to set the amount of memory reserved to the
> > +buddy allocator.
>
> Like for the options in patch 1, here I think you also want to make explicit
> the dependency on CONFIG_LLC_COLORING.

Yep.

> > @@ -1945,6 +1960,155 @@ static unsigned long avail_heap_pages(
> >      return free_pages;
> >  }
> >
> > +/*************************
> > + * COLORED SIDE-ALLOCATOR
> > + *
> > + * Pages are grouped by LLC color in lists which are globally referred to 
> > as the
> > + * color heap. Lists are populated in end_boot_allocator().
> > + * After initialization there will be N lists where N is the number of
> > + * available colors on the platform.
> > + */
> > +static struct page_list_head *__ro_after_init _color_heap;
> > +#define color_heap(color) (&_color_heap[color])
> > +
> > +static unsigned long *__ro_after_init free_colored_pages;
> > +
> > +static unsigned long __initdata buddy_alloc_size =
> > +    MB(CONFIG_BUDDY_ALLOCATOR_SIZE);
> > +size_param("buddy-alloc-size", buddy_alloc_size);
> > +
> > +/* Memory required for buddy allocator to work with colored one */
> > +#ifdef CONFIG_LLC_COLORING
> > +#define domain_num_llc_colors(d) (d)->num_llc_colors
> > +#define domain_llc_color(d, i)   (d)->llc_colors[i]
>
> I'm uncertain whether Misra would insist on an outer pair of parentheses for
> both of these.

Yes, rule 20.7.

> Jan

- Carlo