Re: [linux-pm] [PATCH] hibernation should work ok with memory hotplug

From: Yasunori Goto
Date: Wed Nov 05 2008 - 22:28:46 EST


Hmm.

I think simple mutex is enough. read/write lock is not necessary.
Memory hotplug event is very rare, and there is no requirement of
parallel execution.


> On Thu, 6 Nov 2008 11:07:09 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> > Okay. then we can add "kernel thread for calling add/remove memory" and say
> > "PLEASE WAIT UNTIL HIBERNATION IS READY".
> >
> > I can try that by myself but doesn't have suitable machine....
> > I think I can show you pseudo code in hours. please wait a bit.
> >
>
> How about this one ? as a start step ?
> I have no test environment. So please take this as a sample.
> -Kame
> =
>
> Now, MEMORY_HOTPLUG and HIBERNATION is mutually exclusive in Kconfig.
> That's because
> - memory hotplug changes pgdat/zone/memmap range.
> - hibernation countes # of memory based on pgdate/zone/memmap.
>
> This patch adds rwsemaphore for making them not to run at the same time.
>
> IIUC, add_memory() is called from following places.
> - probe interface
> - acpi handler
> It seems both can sleep. (acpi handler uses sleepable memory allocator etc...)
>
> online/offline handlers also sleeps.
>
> Anyway, the most difficult thing is how to test this.
>
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
>
> mm/Kconfig | 2 -
> mm/memory_hotplug.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 54 insertions(+), 9 deletions(-)
>
> Index: linux-2.6.27.4/mm/memory_hotplug.c
> ===================================================================
> --- linux-2.6.27.4.orig/mm/memory_hotplug.c
> +++ linux-2.6.27.4/mm/memory_hotplug.c
> @@ -31,6 +31,13 @@
>
> #include "internal.h"
>
> +struct rw_semaphore memhp_mutex;
> +
> +static int init_memhp_mutex(void)
> +{
> + init_rwsem(&memhp_mutex);
> +}
> +
> /* add this memory to iomem resource */
> static struct resource *register_memory_resource(u64 start, u64 size)
> {
> @@ -381,6 +388,7 @@ int online_pages(unsigned long pfn, unsi
> int ret;
> struct memory_notify arg;
>
> + down_read(&memhp_mutex);
> arg.start_pfn = pfn;
> arg.nr_pages = nr_pages;
> arg.status_change_nid = -1;
> @@ -392,6 +400,7 @@ int online_pages(unsigned long pfn, unsi
> ret = memory_notify(MEM_GOING_ONLINE, &arg);
> ret = notifier_to_errno(ret);
> if (ret) {
> + up_read(&memhp_mutex);
> memory_notify(MEM_CANCEL_ONLINE, &arg);
> return ret;
> }
> @@ -415,6 +424,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);
> + up_read(&memhp_mutex);
> return ret;
> }
>
> @@ -436,7 +446,7 @@ int online_pages(unsigned long pfn, unsi
>
> if (onlined_pages)
> memory_notify(MEM_ONLINE, &arg);
> -
> + up_read(&memhp_mutex);
> return 0;
> }
> #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
> @@ -477,14 +487,18 @@ int add_memory(int nid, u64 start, u64 s
> struct resource *res;
> int ret;
>
> + down_read(&memhp_mutex);
> res = register_memory_resource(start, size);
> - if (!res)
> + if (!res) {
> + up_read(&memhp_mutex);
> return -EEXIST;
> -
> + }
> if (!node_online(nid)) {
> pgdat = hotadd_new_pgdat(nid, start);
> - if (!pgdat)
> + if (!pgdat) {
> + up_read(&memhp_mutex);
> return -ENOMEM;
> + }
> new_pgdat = 1;
> }
>
> @@ -508,7 +522,7 @@ int add_memory(int nid, u64 start, u64 s
> */
> BUG_ON(ret);
> }
> -
> + up_read(&memhp_mutex);
> return ret;
> error:
> /* rollback pgdat allocation and others */
> @@ -517,10 +531,12 @@ error:
> if (res)
> release_memory_resource(res);
>
> + up_read(&memhp_mutex);
> return ret;
> }
> EXPORT_SYMBOL_GPL(add_memory);
>
> +
> #ifdef CONFIG_MEMORY_HOTREMOVE
> /*
> * A free page on the buddy free lists (not the per-cpu lists) has PageBuddy
> @@ -750,20 +766,23 @@ int offline_pages(unsigned long start_pf
> return -EINVAL;
> if (!IS_ALIGNED(end_pfn, pageblock_nr_pages))
> return -EINVAL;
> +
> /* This makes hotplug much easier...and readable.
> we assume this for now. .*/
> if (!test_pages_in_a_zone(start_pfn, end_pfn))
> return -EINVAL;
>
> + down_read(&memhp_mutex);
> zone = page_zone(pfn_to_page(start_pfn));
> node = zone_to_nid(zone);
> nr_pages = end_pfn - start_pfn;
>
> /* set above range as isolated */
> ret = start_isolate_page_range(start_pfn, end_pfn);
> - if (ret)
> + if (ret) {
> + up_read(&memhp_mutex);
> return ret;
> -
> + }
> arg.start_pfn = start_pfn;
> arg.nr_pages = nr_pages;
> arg.status_change_nid = -1;
> @@ -838,6 +857,7 @@ repeat:
> writeback_set_ratelimit();
>
> memory_notify(MEM_OFFLINE, &arg);
> + up_read(&memhp_mutex);
> return 0;
>
> failed_removal:
> @@ -846,7 +866,7 @@ failed_removal:
> memory_notify(MEM_CANCEL_OFFLINE, &arg);
> /* pushback to free area */
> undo_isolate_page_range(start_pfn, end_pfn);
> -
> + up_read(&memhp_mutex);
> return ret;
> }
> #else
> @@ -856,3 +876,34 @@ int remove_memory(u64 start, u64 size)
> }
> EXPORT_SYMBOL_GPL(remove_memory);
> #endif /* CONFIG_MEMORY_HOTREMOVE */
> +
> +
> +#ifdef CONFIG_HIBERNATION
> +
> +int memhp_hibenation_callback(struct notifier_block *self,
> + unsigned long action,
> + void *arg)
> +{
> + int ret;
> +
> + switch(action) {
> + case PM_HIBERNATION_PREPARE:
> + down_write(&memhp_mutex);
> + break;
> + case PM_POST_HIBERNATION:
> + up_write(&memhp_mutex);
> + break;
> + case PM_RESTORE_PREPARE:
> + down_write(&memhp_mutex);
> + break;
> + case PM_POST_RESTORE:
> + up_write(&memhp_mutex);
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
> +#endif
> +
> +
> Index: linux-2.6.27.4/mm/Kconfig
> ===================================================================
> --- linux-2.6.27.4.orig/mm/Kconfig
> +++ linux-2.6.27.4/mm/Kconfig
> @@ -128,12 +128,9 @@ config SPARSEMEM_VMEMMAP
> config MEMORY_HOTPLUG
> bool "Allow for memory hot-add"
> depends on SPARSEMEM || X86_64_ACPI_NUMA
> - depends on HOTPLUG && !HIBERNATION && ARCH_ENABLE_MEMORY_HOTPLUG
> + depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
> depends on (IA64 || X86 || PPC64 || SUPERH || S390)
>
> -comment "Memory hotplug is currently incompatible with Software Suspend"
> - depends on SPARSEMEM && HOTPLUG && HIBERNATION
> -
> config MEMORY_HOTPLUG_SPARSE
> def_bool y
> depends on SPARSEMEM && MEMORY_HOTPLUG
>

--
Yasunori Goto


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