linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: akpm@linux-foundation.org
To: akpm@linux-foundation.org, anshuman.khandual@arm.com,
	dan.j.williams@intel.com, david@redhat.com, linux-mm@kvack.org,
	mhocko@suse.com, mm-commits@vger.kernel.org, osalvador@suse.de,
	pasha.tatashin@soleen.com, torvalds@linux-foundation.org
Subject: [patch 074/158] mm/hotplug: reorder memblock_[free|remove]() calls in try_remove_memory()
Date: Sat, 30 Nov 2019 17:53:44 -0800	[thread overview]
Message-ID: <20191201015344.HHVU5QQZn%akpm@linux-foundation.org> (raw)

From: Anshuman Khandual <anshuman.khandual@arm.com>
Subject: mm/hotplug: reorder memblock_[free|remove]() calls in try_remove_memory()

Currently during memory hot add procedure, memory gets into memblock
before calling arch_add_memory() which creates its linear mapping.

add_memory_resource() {
	..................
	memblock_add_node()
	..................
	arch_add_memory()
	..................
}

But during memory hot remove procedure, removal from memblock happens
first before its linear mapping gets teared down with arch_remove_memory()
which is not consistent.  Resource removal should happen in reverse order
as they were added.  However this does not pose any problem for now,
unless there is an assumption regarding linear mapping.  One example was a
subtle failure on arm64 platform [1].  Though this has now found a
different solution.

try_remove_memory() {
	..................
	memblock_free()
	memblock_remove()
	..................
	arch_remove_memory()
	..................
}

This changes the sequence of resource removal including memblock and
linear mapping tear down during memory hot remove which will now be the
reverse order in which they were added during memory hot add.  The changed
removal order looks like the following.

try_remove_memory() {
	..................
	arch_remove_memory()
	..................
	memblock_free()
	memblock_remove()
	..................
}

[1] https://patchwork.kernel.org/patch/11127623/

Memory hot remove now works on arm64 without this because a recent commit
60bb462fc7ad ("drivers/base/node.c: simplify
unregister_memory_block_under_nodes()").

This does not fix a serious problem.  It just removes an inconsistency
while freeing resources during memory hot remove which for now does not
pose a real problem.

David mentioned that re-ordering should still make sense for consistency
purpose (removing stuff in the reverse order they were added).  This patch
is now detached from arm64 hot-remove series.

Michal:

: I would just a note that the inconsistency doesn't pose any problem now
: but if somebody makes any assumptions about linear mappings then it could
: get subtly broken like your example for arm64 which has found a different
: solution in the meantime.

Link: http://lkml.kernel.org/r/1569380273-7708-1-git-send-email-anshuman.khandual@arm.com
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/memory_hotplug.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/mm/memory_hotplug.c~mm-hotplug-reorder-memblock_-calls-in-try_remove_memory
+++ a/mm/memory_hotplug.c
@@ -1750,13 +1750,13 @@ static int __ref try_remove_memory(int n
 
 	/* remove memmap entry */
 	firmware_map_remove(start, start + size, "System RAM");
-	memblock_free(start, size);
-	memblock_remove(start, size);
 
 	/* remove memory block devices before removing memory */
 	remove_memory_block_devices(start, size);
 
 	arch_remove_memory(nid, start, size, NULL);
+	memblock_free(start, size);
+	memblock_remove(start, size);
 	__release_memory_resource(start, size);
 
 	try_offline_node(nid);
_


                 reply	other threads:[~2019-12-01  1:53 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20191201015344.HHVU5QQZn%akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=dan.j.williams@intel.com \
    --cc=david@redhat.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=mm-commits@vger.kernel.org \
    --cc=osalvador@suse.de \
    --cc=pasha.tatashin@soleen.com \
    --cc=torvalds@linux-foundation.org \
    /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).