When "hugetlb_free_vmemmap=on" and "memory_hotplug.memmap_on_memory"
are both passed to boot cmdline, the variable of "memmap_on_memory"
will be set to 1 even if the vmemmap pages will not be allocated from
the hotadded memory since the former takes precedence over the latter.
In the next patch, we want to enable or disable the feature of freeing
vmemmap pages of HugeTLB via sysctl. We need a way to know if the
feature of memory_hotplug.memmap_on_memory is enabled when enabling
the feature of freeing vmemmap pages since those two features are not
compatible, however, the variable of "memmap_on_memory" cannot indicate
this nowadays. Do not set "memmap_on_memory" to 1 when both parameters
are passed to cmdline, in this case, "memmap_on_memory" could indicate
if this feature is enabled by the users.
Also introduce mhp_memmap_on_memory() helper to move the definition of
"memmap_on_memory" to the scope of CONFIG_MHP_MEMMAP_ON_MEMORY. In the
next patch, mhp_memmap_on_memory() will also be exported to be used in
hugetlb_vmemmap.c.
Signed-off-by: Muchun Song <[email protected]>
---
mm/memory_hotplug.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 111684878fd9..a6101ae402f9 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -42,14 +42,36 @@
#include "internal.h"
#include "shuffle.h"
+#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
+static int memmap_on_memory_set(const char *val, const struct kernel_param *kp)
+{
+ if (hugetlb_optimize_vmemmap_enabled())
+ return 0;
+ return param_set_bool(val, kp);
+}
+
+static const struct kernel_param_ops memmap_on_memory_ops = {
+ .flags = KERNEL_PARAM_OPS_FL_NOARG,
+ .set = memmap_on_memory_set,
+ .get = param_get_bool,
+};
/*
* memory_hotplug.memmap_on_memory parameter
*/
static bool memmap_on_memory __ro_after_init;
-#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
-module_param(memmap_on_memory, bool, 0444);
+module_param_cb(memmap_on_memory, &memmap_on_memory_ops, &memmap_on_memory, 0444);
MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug");
+
+static inline bool mhp_memmap_on_memory(void)
+{
+ return memmap_on_memory;
+}
+#else
+static inline bool mhp_memmap_on_memory(void)
+{
+ return false;
+}
#endif
enum {
@@ -1263,9 +1285,7 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
* altmap as an alternative source of memory, and we do not exactly
* populate a single PMD.
*/
- return memmap_on_memory &&
- !hugetlb_optimize_vmemmap_enabled() &&
- IS_ENABLED(CONFIG_MHP_MEMMAP_ON_MEMORY) &&
+ return mhp_memmap_on_memory() &&
size == memory_block_size_bytes() &&
IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
@@ -2083,7 +2103,7 @@ static int __ref try_remove_memory(u64 start, u64 size)
* We only support removing memory added with MHP_MEMMAP_ON_MEMORY in
* the same granularity it was added - a single memory block.
*/
- if (memmap_on_memory) {
+ if (mhp_memmap_on_memory()) {
nr_vmemmap_pages = walk_memory_blocks(start, size, NULL,
get_nr_vmemmap_pages_cb);
if (nr_vmemmap_pages) {
--
2.11.0
On 4/29/22 05:18, Muchun Song wrote:
> When "hugetlb_free_vmemmap=on" and "memory_hotplug.memmap_on_memory"
> are both passed to boot cmdline, the variable of "memmap_on_memory"
> will be set to 1 even if the vmemmap pages will not be allocated from
> the hotadded memory since the former takes precedence over the latter.
I had to read that sentence a few times before understanding what it was
trying to say. Not insisting, but how about this instead:
Freeing HugeTLB vmemmap pages is not compatible with allocating memmap on
hot added memory. If "hugetlb_free_vmemmap=on" and
memory_hotplug.memmap_on_memory" are both passed on the kernel command line,
freeing hugetlb pages takes precedence. However, the global variable
memmap_on_memory will still be set to 1, even though we will not try to
allocate memmap on hot added memory.
Not sure if that is more clear or not.
> In the next patch, we want to enable or disable the feature of freeing
> vmemmap pages of HugeTLB via sysctl. We need a way to know if the
> feature of memory_hotplug.memmap_on_memory is enabled when enabling
> the feature of freeing vmemmap pages since those two features are not
> compatible, however, the variable of "memmap_on_memory" cannot indicate
> this nowadays. Do not set "memmap_on_memory" to 1 when both parameters
> are passed to cmdline, in this case, "memmap_on_memory" could indicate
> if this feature is enabled by the users.
>
> Also introduce mhp_memmap_on_memory() helper to move the definition of
> "memmap_on_memory" to the scope of CONFIG_MHP_MEMMAP_ON_MEMORY. In the
> next patch, mhp_memmap_on_memory() will also be exported to be used in
> hugetlb_vmemmap.c.
>
> Signed-off-by: Muchun Song <[email protected]>
> ---
> mm/memory_hotplug.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
No issues with the changes,
Acked-by: Mike Kravetz <[email protected]>
--
Mike Kravetz
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 111684878fd9..a6101ae402f9 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -42,14 +42,36 @@
> #include "internal.h"
> #include "shuffle.h"
>
> +#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
> +static int memmap_on_memory_set(const char *val, const struct kernel_param *kp)
> +{
> + if (hugetlb_optimize_vmemmap_enabled())
> + return 0;
> + return param_set_bool(val, kp);
> +}
> +
> +static const struct kernel_param_ops memmap_on_memory_ops = {
> + .flags = KERNEL_PARAM_OPS_FL_NOARG,
> + .set = memmap_on_memory_set,
> + .get = param_get_bool,
> +};
>
> /*
> * memory_hotplug.memmap_on_memory parameter
> */
> static bool memmap_on_memory __ro_after_init;
> -#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
> -module_param(memmap_on_memory, bool, 0444);
> +module_param_cb(memmap_on_memory, &memmap_on_memory_ops, &memmap_on_memory, 0444);
> MODULE_PARM_DESC(memmap_on_memory, "Enable memmap on memory for memory hotplug");
> +
> +static inline bool mhp_memmap_on_memory(void)
> +{
> + return memmap_on_memory;
> +}
> +#else
> +static inline bool mhp_memmap_on_memory(void)
> +{
> + return false;
> +}
> #endif
>
> enum {
> @@ -1263,9 +1285,7 @@ bool mhp_supports_memmap_on_memory(unsigned long size)
> * altmap as an alternative source of memory, and we do not exactly
> * populate a single PMD.
> */
> - return memmap_on_memory &&
> - !hugetlb_optimize_vmemmap_enabled() &&
> - IS_ENABLED(CONFIG_MHP_MEMMAP_ON_MEMORY) &&
> + return mhp_memmap_on_memory() &&
> size == memory_block_size_bytes() &&
> IS_ALIGNED(vmemmap_size, PMD_SIZE) &&
> IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT));
> @@ -2083,7 +2103,7 @@ static int __ref try_remove_memory(u64 start, u64 size)
> * We only support removing memory added with MHP_MEMMAP_ON_MEMORY in
> * the same granularity it was added - a single memory block.
> */
> - if (memmap_on_memory) {
> + if (mhp_memmap_on_memory()) {
> nr_vmemmap_pages = walk_memory_blocks(start, size, NULL,
> get_nr_vmemmap_pages_cb);
> if (nr_vmemmap_pages) {
On Tue, May 03, 2022 at 05:23:37PM -0700, Mike Kravetz wrote:
> On 4/29/22 05:18, Muchun Song wrote:
> > When "hugetlb_free_vmemmap=on" and "memory_hotplug.memmap_on_memory"
> > are both passed to boot cmdline, the variable of "memmap_on_memory"
> > will be set to 1 even if the vmemmap pages will not be allocated from
> > the hotadded memory since the former takes precedence over the latter.
>
> I had to read that sentence a few times before understanding what it was
> trying to say. Not insisting, but how about this instead:
>
Sorry for my poor English.
> Freeing HugeTLB vmemmap pages is not compatible with allocating memmap on
> hot added memory. If "hugetlb_free_vmemmap=on" and
> memory_hotplug.memmap_on_memory" are both passed on the kernel command line,
> freeing hugetlb pages takes precedence. However, the global variable
> memmap_on_memory will still be set to 1, even though we will not try to
> allocate memmap on hot added memory.
>
> Not sure if that is more clear or not.
>
Clearer than mine.
> Acked-by: Mike Kravetz <[email protected]>
>
Thanks.
On Wed, 4 May 2022 11:37:43 +0800 Muchun Song <[email protected]> wrote:
> > Freeing HugeTLB vmemmap pages is not compatible with allocating memmap on
> > hot added memory. If "hugetlb_free_vmemmap=on" and
> > memory_hotplug.memmap_on_memory" are both passed on the kernel command line,
> > freeing hugetlb pages takes precedence. However, the global variable
> > memmap_on_memory will still be set to 1, even though we will not try to
> > allocate memmap on hot added memory.
> >
> > Not sure if that is more clear or not.
> >
>
> Clearer than mine.
I updated, thanks.