On 26.08.2022 14:51, Carlo Nonato wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -605,6 +605,27 @@ static struct page_info *alloc_col_domheap_page(struct
> domain *d,
> return pg;
> }
>
> +static void dump_col_heap(unsigned char key)
> +{
> + struct page_info *pg;
const and perhaps move into the loop's scope?
> + unsigned long pages;
> + unsigned int color;
> +
> + printk("'%c' pressed -> dumping coloring heap info\n", key);
> +
> + for ( color = 0; color < get_max_colors(); color++ )
> + {
> + printk("Heap[%u]: ", color);
> + pages = 0;
> + page_list_for_each( pg, colored_pages(color) )
> + {
> + BUG_ON(!(page_to_color(pg) == color));
> + pages++;
> + }
This is a very inefficient way for obtaining a count. On a large
system this loop is liable to take excessively long. I'm inclined
to say that even adding a call to process_pending_softirqs() isn't
going to make this work reasonably.
I'm also not convinced of having BUG_ON() in keyhandler functions
which are supposed to only dump state.
> @@ -2853,6 +2874,9 @@ static void cf_check dump_heap(unsigned char key)
> static __init int cf_check register_heap_trigger(void)
> {
> register_keyhandler('H', dump_heap, "dump heap info", 1);
> +#ifdef CONFIG_CACHE_COLORING
> + register_keyhandler('k', dump_col_heap, "dump coloring heap info", 1);
> +#endif
I question the consuming of a separate key for this purpose: What's
wrong with adding the functionality to dump_heap()?
Jan
