From: Mike Kravetz <mike.kravetz@oracle.com>
To: Muchun Song <songmuchun@bytedance.com>,
corbet@lwn.net, akpm@linux-foundation.org, mcgrof@kernel.org,
keescook@chromium.org, yzaikin@google.com, osalvador@suse.de,
david@redhat.com, masahiroy@kernel.org
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, duanxiongchun@bytedance.com,
smuchun@gmail.com
Subject: Re: [PATCH v9 2/4] mm: memory_hotplug: override memmap_on_memory when hugetlb_free_vmemmap=on
Date: Tue, 3 May 2022 17:23:37 -0700 [thread overview]
Message-ID: <3d040faf-7fc1-80a6-c584-aafeff27af18@oracle.com> (raw)
In-Reply-To: <20220429121816.37541-3-songmuchun@bytedance.com>
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 <songmuchun@bytedance.com>
> ---
> mm/memory_hotplug.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
No issues with the changes,
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
--
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) {
next prev parent reply other threads:[~2022-05-04 0:24 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 12:18 [PATCH v9 0/4] add hugetlb_optimize_vmemmap sysctl Muchun Song
2022-04-29 12:18 ` [PATCH v9 1/4] mm: hugetlb_vmemmap: disable hugetlb_optimize_vmemmap when struct page crosses page boundaries Muchun Song
2022-05-03 21:49 ` Mike Kravetz
2022-04-29 12:18 ` [PATCH v9 2/4] mm: memory_hotplug: override memmap_on_memory when hugetlb_free_vmemmap=on Muchun Song
2022-05-04 0:23 ` Mike Kravetz [this message]
2022-05-04 3:37 ` Muchun Song
2022-05-07 21:05 ` Andrew Morton
2022-04-29 12:18 ` [PATCH v9 3/4] mm: hugetlb_vmemmap: use kstrtobool for hugetlb_vmemmap param parsing Muchun Song
2022-05-04 0:42 ` Mike Kravetz
2022-04-29 12:18 ` [PATCH v9 4/4] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap sysctl Muchun Song
2022-05-04 22:12 ` Mike Kravetz
2022-05-05 2:35 ` Muchun Song
2022-05-05 3:36 ` Mike Kravetz
2022-05-05 8:02 ` Muchun Song
2022-05-05 16:48 ` Mike Kravetz
2022-05-06 2:49 ` Muchun Song
2022-05-06 16:50 ` Mike Kravetz
2022-05-07 13:10 ` Muchun Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3d040faf-7fc1-80a6-c584-aafeff27af18@oracle.com \
--to=mike.kravetz@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=corbet@lwn.net \
--cc=david@redhat.com \
--cc=duanxiongchun@bytedance.com \
--cc=keescook@chromium.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=masahiroy@kernel.org \
--cc=mcgrof@kernel.org \
--cc=osalvador@suse.de \
--cc=smuchun@gmail.com \
--cc=songmuchun@bytedance.com \
--cc=yzaikin@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).