Re: [PATCH v8 3/3] mm: reinititalise user and admin reserves ifmemory is added or removed

From: Andrew Morton
Date: Mon Apr 08 2013 - 16:37:22 EST

On Mon, 8 Apr 2013 15:07:38 -0400 Andrew Shewmaker <agshew@xxxxxxxxx> wrote:

> This patch alters the admin and user reserves of the previous patches
> in this series when memory is added or removed.
> If memory is added and the reserves have been eliminated or increased above
> the default max, then we'll trust the admin.
> If memory is removed and there isn't enough free memory, then we
> need to reset the reserves.
> Otherwise keep the reserve set by the admin.
> The reserve reset code is the same as the reserve initialization code.
> Does this sound reasonable to other people? I figured that hot removal
> with too large of memory in the reserves was the most important case
> to get right.

Seems reasonable to me.

I don't understand the magic numbers 1<<13 and 1<<17. How could I?
Please add comments explaining how and why these were chosen.

Your patch adds 400 bytes of unusable code to the
CONFIG_MEMORY_HOTPLUG=n kernel. We have a fix for that in the CPU
hotplug case: register_hotcpu_notifier(). Memory hotplug has its own
hotplug_memory_notifier() but a) it's broken and b) it just doesn't
work! With my gcc-4.4.4, the unused functions are still included in
the .o file.

So I did this:

From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: include/linux/memory.h: implement register_hotmemory_notifier()

When CONFIG_MEMORY_HOTPLUG=n, we don't want the memory-hotplug notifier
handlers to be included in the .o files, for space reasons.

The existing hotplug_memory_notifier() tries to handle this but testing
with gcc-4.4.4 shows that it doesn't work - the hotplug functions are
still present in the .o files.

So implement a new register_hotmemory_notifier() which is a copy of
register_hotcpu_notifier(), and which actually works as desired.
hotplug_memory_notifier() and register_memory_notifier() callsites should
be converted to use this new register_hotmemory_notifier().

While we're there, let's repair the existing hotplug_memory_notifier(): it
simply stomps on the register_memory_notifier() return value, so
well-behaved code cannot check for errors. Apparently non of the existing
callers were well-behaved :(

Cc: Andrew Shewmaker <agshew@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>

include/linux/memory.h | 15 ++++++++++++---
include/linux/notifier.h | 5 ++++-
2 files changed, 16 insertions(+), 4 deletions(-)

diff -puN include/linux/memory.h~include-linux-memoryh-implement-register_hotmemory_notifier include/linux/memory.h
--- a/include/linux/memory.h~include-linux-memoryh-implement-register_hotmemory_notifier
+++ a/include/linux/memory.h
@@ -18,6 +18,7 @@
#include <linux/node.h>
#include <linux/compiler.h>
#include <linux/mutex.h>
+#include <linux/notifier.h>


@@ -127,13 +128,21 @@ enum mem_add_context { BOOT, HOTPLUG };

-#define hotplug_memory_notifier(fn, pri) { \
+#define hotplug_memory_notifier(fn, pri) ({ \
static __meminitdata struct notifier_block fn##_mem_nb =\
{ .notifier_call = fn, .priority = pri }; \
register_memory_notifier(&fn##_mem_nb); \
+#define register_hotmemory_notifier(nb) register_memory_notifier(nb)
+#define unregister_hotmemory_notifier(nb) unregister_memory_notifier(nb)
-#define hotplug_memory_notifier(fn, pri) do { } while (0)
+static inline int hotplug_memory_notifier(notifier_fn_t fn, int priority)
+ return 0;
+/* These aren't inline functions due to a GCC bug. */
+#define register_hotmemory_notifier(nb) ({ (void)(nb); 0; })
+#define unregister_hotmemory_notifier(nb) ({ (void)(nb); })

diff -puN include/linux/notifier.h~include-linux-memoryh-implement-register_hotmemory_notifier include/linux/notifier.h
--- a/include/linux/notifier.h~include-linux-memoryh-implement-register_hotmemory_notifier
+++ a/include/linux/notifier.h
@@ -47,8 +47,11 @@
* runtime initialization.

+typedef int (*notifier_fn_t)(struct notifier_block *nb,
+ unsigned long action, void *data);
struct notifier_block {
- int (*notifier_call)(struct notifier_block *, unsigned long, void *);
+ notifier_fn_t notifier_call;
struct notifier_block __rcu *next;
int priority;

And then I changed your patch thusly:

--- a/mm/mmap.c~mm-reinititalise-user-and-admin-reserves-if-memory-is-added-or-removed-fix
+++ a/mm/mmap.c
@@ -3198,7 +3198,7 @@ static struct notifier_block reserve_mem

int __meminit init_reserve_notifier(void)
- if (register_memory_notifier(&reserve_mem_nb))
+ if (register_hotmemory_notifier(&reserve_mem_nb))
printk("Failed registering memory add/remove notifier for admin reserve");

return 0;

and voila, no more bloat.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at