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

[David Rientjes]

On Tue, 11 Jan 2011, KAMEZAWA Hiroyuki wrote:

> 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>

I think taking lock_memory_hotplug() could have been added after the 
initialization of the struct memory_notify, but it's not really important.

Acked-by: David Rientjes <rientjes@xxxxxxxxxx>

This should be targeted for 2.6.38 to avoid racing with memory hot-remove 
and for the pending patch in the slab tree from Christoph.

> ---
>  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/