Messages in this thread |  | | Date | Wed, 22 Oct 2014 14:32:28 +0900 | From | Yasuaki Ishimatsu <> | Subject | Re: [PATCH] memory-hotplug: Clear pgdat which is allocated by bootmem in try_offline_node() |
| |
(2014/10/22 1:56), Toshi Kani wrote: > On Mon, 2014-10-20 at 19:05 +0900, Yasuaki Ishimatsu wrote: > : >> When hot removing memory, pgdat is set to 0 in try_offline_node(). >> But if the pgdat is allocated by bootmem allocator, the clearing >> step is skipped. And when hot adding the same memory, the uninitialized >> pgdat is reused. But free_area_init_node() chacks wether pgdat is set >
> s/chacks/checks
I'll update it.
> > >> to zero. As a result, free_area_init_node() hits WARN_ON(). >> >> This patch clears pgdat which is allocated by bootmem allocator >> in try_offline_node(). >> >> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >> CC: Zhang Zhen <zhenzhang.zhang@huawei.com> >> CC: Wang Nan <wangnan0@huawei.com> >> CC: Tang Chen <tangchen@cn.fujitsu.com> >> CC: Toshi Kani <toshi.kani@hp.com> >> CC: Dave Hansen <dave.hansen@intel.com> >> CC: David Rientjes <rientjes@google.com> >> >> --- >> mm/memory_hotplug.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c >> index 29d8693..7649f7c 100644 >> --- a/mm/memory_hotplug.c >> +++ b/mm/memory_hotplug.c >> @@ -1943,7 +1943,7 @@ void try_offline_node(int nid) >> >> if (!PageSlab(pgdat_page) && !PageCompound(pgdat_page)) >> /* node data is allocated from boot memory */ >> - return; >> + goto out; >
> Do we still need this if-statement? That is, do we have to skip the > for-loop below even though it checks with is_vmalloc_addr()?
You are right. The if-statement is not necessary. So the issue can be fixed by just removing the if-statement.
I'll post updated patch soon.
Thanks, Yasuaki Ishimatsu
> > Thanks, > -Toshi > > >> /* free waittable in each zone */ >> for (i = 0; i < MAX_NR_ZONES; i++) { >> @@ -1957,6 +1957,7 @@ void try_offline_node(int nid) >> vfree(zone->wait_table); >> } >> >> +out: >> /* >> * Since there is no way to guarentee the address of pgdat/zone is not >> * on stack of any kernel threads or used by other kernel objects > >
|  |