From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "Huang, Ying" <ying.huang@intel.com>,
"david@redhat.com" <david@redhat.com>
Cc: "Hocko, Michal" <mhocko@suse.com>,
"Jiang, Dave" <dave.jiang@intel.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"osalvador@suse.de" <osalvador@suse.de>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Williams, Dan J" <dan.j.williams@intel.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
"aneesh.kumar@linux.ibm.com" <aneesh.kumar@linux.ibm.com>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"jmoyer@redhat.com" <jmoyer@redhat.com>,
"Jonathan.Cameron@Huawei.com" <Jonathan.Cameron@Huawei.com>
Subject: Re: [PATCH v5 1/2] mm/memory_hotplug: split memmap_on_memory requests across memblocks
Date: Thu, 12 Oct 2023 05:53:20 +0000 [thread overview]
Message-ID: <f0d385f1c1961a17499e5acccf3ae7cdadb942cb.camel@intel.com> (raw)
In-Reply-To: <831b9b12-08fe-f5dc-f21d-83284b0aee8a@redhat.com>
On Mon, 2023-10-09 at 17:04 +0200, David Hildenbrand wrote:
> On 07.10.23 10:55, Huang, Ying wrote:
> > Vishal Verma <vishal.l.verma@intel.com> writes:
> >
> > > @@ -2167,47 +2221,28 @@ static int __ref try_remove_memory(u64 start, u64 size)
> > > if (rc)
> > > return rc;
> > >
> > > + mem_hotplug_begin();
> > > +
> > > /*
> > > - * We only support removing memory added with MHP_MEMMAP_ON_MEMORY in
> > > - * the same granularity it was added - a single memory block.
> > > + * For memmap_on_memory, the altmaps could have been added on
> > > + * a per-memblock basis. Loop through the entire range if so,
> > > + * and remove each memblock and its altmap.
> > > */
> > > if (mhp_memmap_on_memory()) {
> >
> > IIUC, even if mhp_memmap_on_memory() returns true, it's still possible
> > that the memmap is put in DRAM after [2/2]. So that,
> > arch_remove_memory() are called for each memory block unnecessarily. Can
> > we detect this (via altmap?) and call remove_memory_block_and_altmap()
> > for the whole range?
>
> Good point. We should handle memblock-per-memblock onny if we have to
> handle the altmap. Otherwise, just call a separate function that doesn't
> care about -- e.g., called remove_memory_blocks_no_altmap().
>
> We could simply walk all memory blocks and make sure either all have an
> altmap or none has an altmap. If there is a mix, we should bail out with
> WARN_ON_ONCE().
>
Ok I think I follow - based on both of these threads, here's my
understanding in an incremental diff from the original patches (may not
apply directly as I've already committed changes from the other bits of
feedback - but this should provide an idea of the direction) -
---
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 507291e44c0b..30addcb063b4 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2201,6 +2201,40 @@ static void __ref remove_memory_block_and_altmap(u64 start, u64 size)
}
}
+static bool memblocks_have_altmaps(u64 start, u64 size)
+{
+ unsigned long memblock_size = memory_block_size_bytes();
+ u64 num_altmaps = 0, num_no_altmaps = 0;
+ struct memory_block *mem;
+ u64 cur_start;
+ int rc = 0;
+
+ if (!mhp_memmap_on_memory())
+ return false;
+
+ for (cur_start = start; cur_start < start + size;
+ cur_start += memblock_size) {
+ if (walk_memory_blocks(cur_start, memblock_size, &mem,
+ test_has_altmap_cb))
+ num_altmaps++;
+ else
+ num_no_altmaps++;
+ }
+
+ if (!num_altmaps && num_no_altmaps > 0)
+ return false;
+
+ if (!num_no_altmaps && num_altmaps > 0)
+ return true;
+
+ /*
+ * If there is a mix of memblocks with and without altmaps,
+ * something has gone very wrong. WARN and bail.
+ */
+ WARN_ONCE(1, "memblocks have a mix of missing and present altmaps");
+ return false;
+}
+
static int __ref try_remove_memory(u64 start, u64 size)
{
int rc, nid = NUMA_NO_NODE;
@@ -2230,7 +2264,7 @@ static int __ref try_remove_memory(u64 start, u64 size)
* a per-memblock basis. Loop through the entire range if so,
* and remove each memblock and its altmap.
*/
- if (mhp_memmap_on_memory()) {
+ if (mhp_memmap_on_memory() && memblocks_have_altmaps(start, size)) {
unsigned long memblock_size = memory_block_size_bytes();
u64 cur_start;
@@ -2239,7 +2273,8 @@ static int __ref try_remove_memory(u64 start, u64 size)
remove_memory_block_and_altmap(cur_start,
memblock_size);
} else {
- remove_memory_block_and_altmap(start, size);
+ remove_memory_block_devices(start, size);
+ arch_remove_memory(start, size, NULL);
}
if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) {
next prev parent reply other threads:[~2023-10-12 5:53 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-05 18:31 [PATCH v5 0/2] mm: use memmap_on_memory semantics for dax/kmem Vishal Verma
2023-10-05 18:31 ` [PATCH v5 1/2] mm/memory_hotplug: split memmap_on_memory requests across memblocks Vishal Verma
2023-10-05 21:20 ` Dan Williams
2023-10-06 16:46 ` Verma, Vishal L
2023-10-06 12:52 ` David Hildenbrand
2023-10-06 22:01 ` Verma, Vishal L
2023-10-09 15:15 ` David Hildenbrand
2023-10-07 8:55 ` Huang, Ying
2023-10-09 15:04 ` David Hildenbrand
2023-10-12 5:53 ` Verma, Vishal L [this message]
2023-10-12 8:40 ` David Hildenbrand
2023-10-16 18:19 ` Verma, Vishal L
2023-10-05 18:31 ` [PATCH v5 2/2] dax/kmem: allow kmem to add memory with memmap_on_memory Vishal Verma
2023-10-05 21:16 ` Dan Williams
2023-10-17 0:31 ` Verma, Vishal L
2023-10-17 5:18 ` Huang, Ying
2023-10-17 5:44 ` Verma, Vishal L
2023-10-17 5:54 ` Huang, Ying
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=f0d385f1c1961a17499e5acccf3ae7cdadb942cb.camel@intel.com \
--to=vishal.l.verma@intel.com \
--cc=Jonathan.Cameron@Huawei.com \
--cc=akpm@linux-foundation.org \
--cc=aneesh.kumar@linux.ibm.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=dave.jiang@intel.com \
--cc=david@redhat.com \
--cc=jmoyer@redhat.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=nvdimm@lists.linux.dev \
--cc=osalvador@suse.de \
--cc=ying.huang@intel.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 read-only IMAP folder(s) and NNTP newsgroup(s).