Re: [PATCH v12 09/12] xen: add cache coloring allocator for domains

[Michal Orzel <michal.orzel@xxxxxxx>]

On 13/12/2024 17:28, Carlo Nonato wrote:
> 
> 
> Add a new memory page allocator that implements the cache coloring mechanism.
> The allocation algorithm enforces equal frequency distribution of cache
> partitions, following the coloring configuration of a domain. This allows
> for an even utilization of cache sets for every domain.
> 
> Pages are stored in a color-indexed array of lists. Those lists are filled
> by a simple init function which computes the color of each page.
> When a domain requests a page, the allocator extracts the page from the list
> with the maximum number of free pages among those that the domain can access,
> given its coloring configuration.
> 
> The allocator can only handle requests of order-0 pages. This allows for
> easier implementation and since cache coloring targets only embedded systems,
> it's assumed not to be a major problem.
> 
> The buddy allocator must coexist with the colored one because the Xen heap
> isn't colored. For this reason a new Kconfig option and a command line
> parameter are added to let the user set the amount of memory reserved for
> the buddy allocator. Even when cache coloring is enabled, this memory
> isn't managed by the colored allocator.
> 
> Colored heap information is dumped in the dump_heap() debug-key function.
> 
> Based on original work from: Luca Miccio <lucmiccio@xxxxxxxxx>
> 
> Signed-off-by: Marco Solieri <marco.solieri@xxxxxxxxxxxxxxx>
> Signed-off-by: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v12:
> - fixed #ifdef CONFIG_LLC_COLORING in init_color_heap_pages()
> v11:
> - CONFIG_BUDDY_ALLOCATOR_SIZE depends on CONFIG_LLC_COLORING
> - buddy_alloc_size is defined only if CONFIG_LLC_COLORING
> - buddy-alloc-size param is parsed only if CONFIG_LLC_COLORING
> v10:
> - stated explicit dependency on CONFIG_LLC_COLORING for buddy-alloc-size
> - fix for MISRA rule 20.7 parenthesis
> v9:
> - added ASSERT(order == 0) when freeing a colored page
> - moved buddy_alloc_size initialization logic in Kconfig
> v8:
> - requests that uses MEMF_* flags that can't be served are now going to fail
> - free_color_heap_page() is called directly from free_heap_pages()
> v7:
> - requests to alloc_color_heap_page() now fail if MEMF_bits is used
> v6:
> - colored allocator functions are now static
> v5:
> - Carlo Nonato as the new author
> - the colored allocator balances color usage for each domain and it searches
>   linearly only in the number of colors (FIXME removed)
> - addedd scrub functionality
> - removed stub functions (still requires some macro definition)
> - addr_to_color turned to mfn_to_color for easier operations
> - removed BUG_ON in init_color_heap_pages() in favor of panic()
> - only non empty page lists are logged in dump_color_heap()
> v4:
> - moved colored allocator code after buddy allocator because it now has
>   some dependencies on buddy functions
> - buddy_alloc_size is now used only by the colored allocator
> - fixed a bug that allowed the buddy to merge pages when they were colored
> - free_color_heap_page() now calls mark_page_free()
> - free_color_heap_page() uses of the frametable array for faster searches
> - added FIXME comment for the linear search in free_color_heap_page()
> - removed alloc_color_domheap_page() to let the colored allocator exploit
>   some more buddy allocator code
> - alloc_color_heap_page() now allocs min address pages first
> - reduced the mess in end_boot_allocator(): use the first loop for
>   init_color_heap_pages()
> - fixed page_list_add_prev() (list.h) since it was doing the opposite of
>   what it was supposed to do
> - fixed page_list_add_prev() (non list.h) to check also for next existence
> - removed unused page_list_add_next()
> - moved p2m code in another patch
> ---
>  docs/misc/cache-coloring.rst      |  37 ++++++
>  docs/misc/xen-command-line.pandoc |  14 +++
>  xen/arch/arm/include/asm/mm.h     |   5 +
[...]

> +#ifdef CONFIG_LLC_COLORING
> +/* Page is cache colored */
> +#define _PGC_colored      PG_shift(4)
NIT: do we really need to define it if it's not used? I don't like this concept 
but
I quickly checked new arches and they also copied some flags that are not used, 
so
maybe it's the proper way to go.

Acked-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal