[PATCH] one more lock on memory hotplug (Re: [PATCH] slub: Fixsysfs circular locking dependency

[KAMEZAWA Hiroyuki]

On Tue, 11 Jan 2011 08:37:29 +0200
Pekka Enberg <penberg@xxxxxxxxxx> wrote:

> On 1/10/11 10:30 PM, David Rientjes wrote:
> > On Mon, 10 Jan 2011, Christoph Lameter wrote:
> >
> >> New patch that just covers the slub changes.
> >>
> >> Subject: slub: Avoid use of slub_lock in show_slab_objects()
> >>
> >> The purpose of the locking is to prevent removal and additions
> >> of nodes when statistics are gathered for a slab cache. So we
> >> need to avoid racing with memory hotplug functionality.
> >>
> >> It is enough to take the memory hotplug locks there instead
> >> of the slub_lock.
> >>
> >> online_pages() currently does not acquire the memory_hotplug
> >> lock. Another patch will be submitted by the memory hotplug
> >> authors to take the memory hotplug lock and describe the
> >> uses of the memory hotplug lock to protect against
> >> adding and removal of nodes from non hotplug data structures.
> >>
> >> Signed-off-by: Christoph Lameter<cl@xxxxxxxxx>
> > Acked-by: David Rientjes<rientjes@xxxxxxxxxx>
> 
> Is this safe to be applied without the other hotplug parts?
> 
> 

How about this ? 
==
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>

Now, memory_hotplug_(un)lock() is used for add/remove/offline pages
for avoiding races with hibernation. But this should be held in
online_pages(), too. It seems asymmetric.

There are cases where one has to avoid a race with memory hotplug
notifier and his own local code, and hotplug v.s. hotplug.
This will add a generic solution for avoiding races. In other view,
having lock here has no big impacts. online pages is tend to be
done by udev script at el against each memory section one by one.

Then, it's better to have lock here, too.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
---
 include/linux/memory_hotplug.h |    6 ++++++
 mm/memory_hotplug.c            |    4 ++++
 2 files changed, 10 insertions(+)

Index: linux-2.6.37.org/include/linux/memory_hotplug.h
===================================================================
--- linux-2.6.37.org.orig/include/linux/memory_hotplug.h
+++ linux-2.6.37.org/include/linux/memory_hotplug.h
@@ -161,6 +161,12 @@ extern void register_page_bootmem_info_n
 extern void put_page_bootmem(struct page *page);
 #endif
 
+/*
+ * Lock for memory hotplug guarantees 1) all callbacks for memory hotplug
+ * notifier will be called under this. 2) offline/online/add/remove memory
+ * will not run simultaneously.
+ */
+
 void lock_memory_hotplug(void);
 void unlock_memory_hotplug(void);
 
Index: linux-2.6.37.org/mm/memory_hotplug.c
===================================================================
--- linux-2.6.37.org.orig/mm/memory_hotplug.c
+++ linux-2.6.37.org/mm/memory_hotplug.c
@@ -407,6 +407,7 @@ int online_pages(unsigned long pfn, unsi
 	int ret;
 	struct memory_notify arg;
 
+	lock_memory_hotplug();
 	arg.start_pfn = pfn;
 	arg.nr_pages = nr_pages;
 	arg.status_change_nid = -1;
@@ -419,6 +420,7 @@ int online_pages(unsigned long pfn, unsi
 	ret = notifier_to_errno(ret);
 	if (ret) {
 		memory_notify(MEM_CANCEL_ONLINE, &arg);
+		unlock_memory_hotplug();
 		return ret;
 	}
 	/*
@@ -443,6 +445,7 @@ int online_pages(unsigned long pfn, unsi
 		printk(KERN_DEBUG "online_pages %lx at %lx failed\n",
 			nr_pages, pfn);
 		memory_notify(MEM_CANCEL_ONLINE, &arg);
+		unlock_memory_hotplug();
 		return ret;
 	}
 
@@ -467,6 +470,7 @@ int online_pages(unsigned long pfn, unsi
 
 	if (onlined_pages)
 		memory_notify(MEM_ONLINE, &arg);
+	unlock_memory_hotplug();
 
 	return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/