Re: [PATCH v3 4/7] mm/hotplug: Allow pageblock alignment via altmap reservation

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

David Hildenbrand <david@redhat.com> writes:

> On 12.07.23 05:16, Aneesh Kumar K V wrote:
>> On 7/11/23 10:49 PM, David Hildenbrand wrote:
>>> On 11.07.23 06:48, Aneesh Kumar K.V wrote:
>>>> Add a new kconfig option that can be selected if we want to allow
>>>> pageblock alignment by reserving pages in the vmemmap altmap area.
>>>> This implies we will be reserving some pages for every memoryblock
>>>> This also allows the memmap on memory feature to be widely useful
>>>> with different memory block size values.
>>>
>>> "reserving pages" is a nice way of saying "wasting memory". :) Let's spell that out.
>>>
>>> I think we have to find a better name for this, and I think we should have a toggle similar to memory_hotplug.memmap_on_memory. This should be an admin decision, not some kernel config option.
>>>
>>>
>>> memory_hotplug.force_memmap_on_memory
>>>
>>> "Enable the memmap on memory feature even if it could result in memory waste due to memmap size limitations. For example, if the memmap for a memory block requires 1 MiB, but the pageblock size is 2 MiB, 1 MiB
>>> of hotplugged memory will be wasted. Note that there are still cases where the feature cannot be enforced: for example, if the memmap is smaller than a single page, or if the architecture does not support the forced mode in all configurations."
>>>
>>> Thoughts?
>>>
>> 
>> With module parameter, do we still need the Kconfig option?
>
> No.
>
> Sleeping over this, maybe we can convert the existing 
> memory_hotplug.memmap_on_memory parameter to also accept "force".
>

How about this? 

modified   mm/memory_hotplug.c
@@ -45,13 +45,67 @@
 /*
  * memory_hotplug.memmap_on_memory parameter
  */
-static bool memmap_on_memory __ro_after_init;
-module_param(memmap_on_memory, bool, 0444);
-MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug");
+enum {
+	MEMMAP_ON_MEMORY_DISABLE = 0,
+	MEMMAP_ON_MEMORY_ENABLE,
+	FORCE_MEMMAP_ON_MEMORY,
+};
+static int memmap_mode __read_mostly = MEMMAP_ON_MEMORY_DISABLE;
+static const char *memmap_on_memory_to_str[] = {
+	[MEMMAP_ON_MEMORY_DISABLE]  = "disable",
+	[MEMMAP_ON_MEMORY_ENABLE]   = "enable",
+	[FORCE_MEMMAP_ON_MEMORY]    = "force",
+};
+
+static inline unsigned long memory_block_align_base(unsigned long size)
+{
+	if (memmap_mode == FORCE_MEMMAP_ON_MEMORY) {
+		unsigned long align;
+		unsigned long nr_vmemmap_pages = size >> PAGE_SHIFT;
+		unsigned long vmemmap_size;
+
+		vmemmap_size = DIV_ROUND_UP(nr_vmemmap_pages * sizeof(struct page), PAGE_SIZE);
+		align = pageblock_align(vmemmap_size) - vmemmap_size;
+		return align;
+	} else
+		return 0;
+}
+
+static int set_memmap_mode(const char *val, const struct kernel_param *kp)
+{
+	int ret = sysfs_match_string(memmap_on_memory_to_str, val);
+
+	if (ret < 0)
+		return ret;
+	*((int *)kp->arg) = ret;
+	if (ret == FORCE_MEMMAP_ON_MEMORY) {
+		pr_info("Memory hotplug will reserve %ld pages in each memory block\n",
+			memory_block_align_base(memory_block_size_bytes()));
+	}
+	return 0;
+}
+
+static int get_memmap_mode(char *buffer, const struct kernel_param *kp)
+{
+	return sprintf(buffer, "%s\n", memmap_on_memory_to_str[*((int *)kp->arg)]);
+}
+
+static const struct kernel_param_ops memmap_mode_ops = {
+	.set = set_memmap_mode,
+	.get = get_memmap_mode,
+};
+module_param_cb(memmap_on_memory, &memmap_mode_ops, &memmap_mode, 0644);
+MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug\n"
+	"With value \"force\" it could result in memory waste due to memmap size limitations \n"
+	"For example, if the memmap for a memory block requires 1 MiB, but the pageblock \n"
+	"size is 2 MiB, 1 MiB of hotplugged memory will be wasted. Note that there are \n"
+	"still cases where the feature cannot be enforced: for example, if the memmap is \n"
+	"smaller than a single page, or if the architecture does not support the forced \n"
+	"mode in all configurations. (disable/enable/force)");
 
 static inline bool mhp_memmap_on_memory(void)
 {
-	return memmap_on_memory;
+	return !!memmap_mode;
 }
 #else

We can also enable runtime enable/disable/force the feature. We just
need to make sure on try_remove_memory we lookup for altmap correctly.