[20/36] xen/common: introduce buddy required reservation

Commit Message

Marco Solieri March 4, 2022, 5:46 p.m. UTC

From: Luca Miccio <[e-mail]>

When cache coloring is enabled, a certain amount of memory is reserved
for buddy allocation because current coloring implementation does not
support Xen heap memory. As of this commit, the colored allocator is used
for dom0, domUs, while the buddy manages only Xen memory. The memory
reserved to the buddy is thus lowered to a reasonably small value.
Introduce a new variable that specifies the amount of memory reserved
for the buddy allocator.
The current default value will be enough even when we will add
coloring for Xen in the following patches.

Signed-off-by: Luca Miccio <[e-mail]>
Signed-off-by: Marco Solieri <[e-mail]>
---
 xen/common/page_alloc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Jan Beulich March 9, 2022, 2:45 p.m. UTC | #1

On 04.03.2022 18:46, Marco Solieri wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -230,6 +230,13 @@ static bool __read_mostly scrub_debug;
>  #define scrub_debug    false
>  #endif
>  
> +#ifdef CONFIG_COLORING
> +/* Minimum size required for buddy allocator to work with colored one */
> +unsigned long buddy_required_size __read_mostly = MB(64);
> +#else
> +unsigned long buddy_required_size __read_mostly = 0;
> +#endif

Please avoid such redundancy when possible. Here perhaps easiest
by having the value come from Kconfig. By giving that separate
option a prompt, it would even become configurable at build time.

> @@ -678,6 +685,13 @@ static void dump_col_heap(unsigned char key)
>  
>      printk("Total number of pages: %lu\n", total_avail_col_pages);
>  }
> +static int __init parse_buddy_required_size(const char *s)
> +{
> +    buddy_required_size = simple_strtoull(s, &s, 0);
> +
> +    return *s ? -EINVAL : 0;
> +}
> +custom_param("buddy_size", parse_buddy_required_size);

Why not integer_param() or, even better fitting the purpose,
size_param()? Also (I may have said so elsewhere already) please
prefer - over _ in new command line option names. And of course
the name needs to be unambiguous enough for it to be easy to
associate the purpose.

Jan

Jan Beulich March 9, 2022, 2:47 p.m. UTC | #2

On 09.03.2022 15:45, Jan Beulich wrote:
> On 04.03.2022 18:46, Marco Solieri wrote:
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -230,6 +230,13 @@ static bool __read_mostly scrub_debug;
>>  #define scrub_debug    false
>>  #endif
>>  
>> +#ifdef CONFIG_COLORING
>> +/* Minimum size required for buddy allocator to work with colored one */
>> +unsigned long buddy_required_size __read_mostly = MB(64);
>> +#else
>> +unsigned long buddy_required_size __read_mostly = 0;
>> +#endif
> 
> Please avoid such redundancy when possible. Here perhaps easiest
> by having the value come from Kconfig. By giving that separate
> option a prompt, it would even become configurable at build time.

Oh, and: Why is this not static? And without seeing what it's going
to be used for it's quite hard to judge whether the initial value
chosen is actually sufficient. I could imagine that this would
rather want to be derived from total memory size.

Jan

Patch

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 82f6e8330a..fffa438029 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -230,6 +230,13 @@  static bool __read_mostly scrub_debug;
 #define scrub_debug    false
 #endif
 
+#ifdef CONFIG_COLORING
+/* Minimum size required for buddy allocator to work with colored one */
+unsigned long buddy_required_size __read_mostly = MB(64);
+#else
+unsigned long buddy_required_size __read_mostly = 0;
+#endif
+
 /*
  * Bit width of the DMA heap -- used to override NUMA-node-first.
  * allocation strategy, which can otherwise exhaust low memory.
@@ -678,6 +685,13 @@  static void dump_col_heap(unsigned char key)
 
     printk("Total number of pages: %lu\n", total_avail_col_pages);
 }
+static int __init parse_buddy_required_size(const char *s)
+{
+    buddy_required_size = simple_strtoull(s, &s, 0);
+
+    return *s ? -EINVAL : 0;
+}
+custom_param("buddy_size", parse_buddy_required_size);
 #else /* !CONFIG_COLORING */
 #define init_col_heap_pages(x, y) init_heap_pages(x, y)