[PATCH 1/2] mm: add private lock to serialize memory hotplug operations

From: Heiko Carstens
Date: Thu Mar 09 2017 - 08:08:35 EST

Commit bfc8c90139eb ("mem-hotplug: implement get/put_online_mems")
introduced new functions get/put_online_mems() and
mem_hotplug_begin/end() in order to allow similar semantics for memory
hotplug like for cpu hotplug.

The corresponding functions for cpu hotplug are get/put_online_cpus()
and cpu_hotplug_begin/done() for cpu hotplug.

The commit however missed to introduce functions that would serialize
memory hotplug operations like they are done for cpu hotplug with

This basically leaves mem_hotplug.active_writer unprotected and allows
concurrent writers to modify it, which may lead to problems as
outlined by commit f931ab479dd2 ("mm: fix devm_memremap_pages crash,
use mem_hotplug_{begin, done}").

That commit was extended again with commit b5d24fda9c3d ("mm,
devm_memremap_pages: hold device_hotplug lock over mem_hotplug_{begin,
done}") which serializes memory hotplug operations for some call
sites by using the device_hotplug lock.

In addition with commit 3fc21924100b ("mm: validate device_hotplug is
held for memory hotplug") a sanity check was added to
mem_hotplug_begin() to verify that the device_hotplug lock is held.

This in turn triggers the following warning on s390:

WARNING: CPU: 6 PID: 1 at drivers/base/core.c:643 assert_held_device_hotplug+0x4a/0x58
Call Trace:

One possible fix would be to add more lock_device_hotplug() and
unlock_device_hotplug() calls around each call site of
mem_hotplug_begin/end(). But that would give the device_hotplug lock
additional semantics it better should not have (serialize memory
hotplug operations).

Instead add a new memory_add_remove_lock which has the similar
semantics like cpu_add_remove_lock for cpu hotplug.

To keep things hopefully a bit easier the lock will be locked and
unlocked within the mem_hotplug_begin/end() functions.

Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
Cc: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
Cc: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
Cc: Gerald Schaefer <gerald.schaefer@xxxxxxxxxx>
Cc: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
Reported-by: Sebastian Ott <sebott@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
kernel/memremap.c | 4 ----
mm/memory_hotplug.c | 6 +++++-
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/memremap.c b/kernel/memremap.c
index 06123234f118..07e85e5229da 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -247,11 +247,9 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
align_start = res->start & ~(SECTION_SIZE - 1);
align_size = ALIGN(resource_size(res), SECTION_SIZE);

- lock_device_hotplug();
arch_remove_memory(align_start, align_size);
- unlock_device_hotplug();

untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
@@ -364,11 +362,9 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
if (error)
goto err_pfn_remap;

- lock_device_hotplug();
error = arch_add_memory(nid, align_start, align_size, true);
- unlock_device_hotplug();
if (error)
goto err_add_memory;

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 295479b792ec..6fa7208bcd56 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -125,9 +125,12 @@ void put_online_mems(void)


+/* Serializes write accesses to mem_hotplug.active_writer. */
+static DEFINE_MUTEX(memory_add_remove_lock);
void mem_hotplug_begin(void)
- assert_held_device_hotplug();
+ mutex_lock(&memory_add_remove_lock);

mem_hotplug.active_writer = current;

@@ -147,6 +150,7 @@ void mem_hotplug_done(void)
mem_hotplug.active_writer = NULL;
+ mutex_unlock(&memory_add_remove_lock);

/* add this memory to iomem resource */