David Hildenbrand <david@xxxxxxxxxx> 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.