| Message ID | 4FEA9DB1.7010303@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote: > When offline_pages() is called to offlined memory, the function fails since > all memory has been offlined. In this case, the function should succeed. > The patch adds the check function into offline_pages(). You miss such case: some pages are online, while some pages are offline. offline_pages() will fail too in such case. Thanks Wen Congyang > > CC: Len Brown <len.brown@intel.com> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> > CC: Paul Mackerras <paulus@samba.org> > CC: Christoph Lameter <cl@linux.com> > Cc: Minchan Kim <minchan.kim@gmail.com> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > CC: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> > > --- > drivers/base/memory.c | 20 ++++++++++++++++++++ > include/linux/memory.h | 1 + > mm/memory_hotplug.c | 5 +++++ > 3 files changed, 26 insertions(+) > > Index: linux-3.5-rc4/drivers/base/memory.c > =================================================================== > --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 > +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 > @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( > } > EXPORT_SYMBOL(unregister_memory_isolate_notifier); > > +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) > +{ > + struct memory_block *mem; > + struct mem_section *section; > + unsigned long pfn, section_nr; > + > + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { > + section_nr = pfn_to_section_nr(pfn); > + section = __nr_to_section(section_nr); > + mem = find_memory_block(section); > + if (!mem) > + continue; > + if (mem->state == MEM_OFFLINE) > + continue; > + return false; > + } > + > + return true; > +} > + > /* > * register_memory - Setup a sysfs device for a memory block > */ > Index: linux-3.5-rc4/include/linux/memory.h > =================================================================== > --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 > +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 > @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne > extern struct memory_block *find_memory_block_hinted(struct mem_section *, > struct memory_block *); > extern struct memory_block *find_memory_block(struct mem_section *); > +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); > #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) > enum mem_add_context { BOOT, HOTPLUG }; > #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ > Index: linux-3.5-rc4/mm/memory_hotplug.c > =================================================================== > --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 > +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 > @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned > > lock_memory_hotplug(); > > + if (memory_is_offline(start_pfn, end_pfn)) { > + ret = 0; > + goto out; > + } > + > zone = page_zone(pfn_to_page(start_pfn)); > node = zone_to_nid(zone); > nr_pages = end_pfn - start_pfn; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Hi Wen, 2012/06/27 17:49, Wen Congyang wrote: > At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote: >> When offline_pages() is called to offlined memory, the function fails since >> all memory has been offlined. In this case, the function should succeed. >> The patch adds the check function into offline_pages(). > > You miss such case: some pages are online, while some pages are offline. > offline_pages() will fail too in such case. You are right. But current code fails, when the function is called to offline memory. In this case, the function should succeed. So the patch confirms whether the memory was offlined or not. And if memory has already been offlined, offline_pages return 0. Thanks, Yasuaki Ishimatsu > > Thanks > Wen Congyang > >> >> CC: Len Brown <len.brown@intel.com> >> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> CC: Paul Mackerras <paulus@samba.org> >> CC: Christoph Lameter <cl@linux.com> >> Cc: Minchan Kim <minchan.kim@gmail.com> >> CC: Andrew Morton <akpm@linux-foundation.org> >> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> CC: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >> >> --- >> drivers/base/memory.c | 20 ++++++++++++++++++++ >> include/linux/memory.h | 1 + >> mm/memory_hotplug.c | 5 +++++ >> 3 files changed, 26 insertions(+) >> >> Index: linux-3.5-rc4/drivers/base/memory.c >> =================================================================== >> --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 >> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 >> @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( >> } >> EXPORT_SYMBOL(unregister_memory_isolate_notifier); >> >> +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) >> +{ >> + struct memory_block *mem; >> + struct mem_section *section; >> + unsigned long pfn, section_nr; >> + >> + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >> + section_nr = pfn_to_section_nr(pfn); >> + section = __nr_to_section(section_nr); >> + mem = find_memory_block(section); >> + if (!mem) >> + continue; >> + if (mem->state == MEM_OFFLINE) >> + continue; >> + return false; >> + } >> + >> + return true; >> +} >> + >> /* >> * register_memory - Setup a sysfs device for a memory block >> */ >> Index: linux-3.5-rc4/include/linux/memory.h >> =================================================================== >> --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 >> +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 >> @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne >> extern struct memory_block *find_memory_block_hinted(struct mem_section *, >> struct memory_block *); >> extern struct memory_block *find_memory_block(struct mem_section *); >> +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); >> #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) >> enum mem_add_context { BOOT, HOTPLUG }; >> #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> =================================================================== >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 >> @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned >> >> lock_memory_hotplug(); >> >> + if (memory_is_offline(start_pfn, end_pfn)) { >> + ret = 0; >> + goto out; >> + } >> + >> zone = page_zone(pfn_to_page(start_pfn)); >> node = zone_to_nid(zone); >> nr_pages = end_pfn - start_pfn; >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
On Wed, Jun 27, 2012 at 1:44 AM, Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote: > When offline_pages() is called to offlined memory, the function fails since > all memory has been offlined. In this case, the function should succeed. > The patch adds the check function into offline_pages(). I don't understand your point. I think following misoperation should fail. Otherwise administrator have no way to know their fault. $ echo offline > memoryN/state $ echo offline > memoryN/state In general, we don't like to ignore an error except the standard require it. > > CC: Len Brown <len.brown@intel.com> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> > CC: Paul Mackerras <paulus@samba.org> > CC: Christoph Lameter <cl@linux.com> > Cc: Minchan Kim <minchan.kim@gmail.com> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > CC: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> > > --- > drivers/base/memory.c | 20 ++++++++++++++++++++ > include/linux/memory.h | 1 + > mm/memory_hotplug.c | 5 +++++ > 3 files changed, 26 insertions(+) > > Index: linux-3.5-rc4/drivers/base/memory.c > =================================================================== > --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 > +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 > @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( > } > EXPORT_SYMBOL(unregister_memory_isolate_notifier); > > +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) I dislike this function name. 'memory' is too vague to me. > +{ > + struct memory_block *mem; > + struct mem_section *section; > + unsigned long pfn, section_nr; > + > + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { > + section_nr = pfn_to_section_nr(pfn); > + section = __nr_to_section(section_nr); > + mem = find_memory_block(section); This seems to have strong sparse dependency. Hm, I wonder why memory-hotplug.c can enable when X86_64_ACPI_NUMA. # eventually, we can have this option just 'select SPARSEMEM' config MEMORY_HOTPLUG bool "Allow for memory hot-add" depends on SPARSEMEM || X86_64_ACPI_NUMA > + if (!mem) > + continue; > + if (mem->state == MEM_OFFLINE) > + continue; > + return false; > + } > + > + return true; > +} > + > /* > * register_memory - Setup a sysfs device for a memory block > */ > Index: linux-3.5-rc4/include/linux/memory.h > =================================================================== > --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 > +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 > @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne > extern struct memory_block *find_memory_block_hinted(struct mem_section *, > struct memory_block *); > extern struct memory_block *find_memory_block(struct mem_section *); > +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); > #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) > enum mem_add_context { BOOT, HOTPLUG }; > #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ > Index: linux-3.5-rc4/mm/memory_hotplug.c > =================================================================== > --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 > +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 > @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned > > lock_memory_hotplug(); > > + if (memory_is_offline(start_pfn, end_pfn)) { > + ret = 0; > + goto out; > + } > + > zone = page_zone(pfn_to_page(start_pfn)); > node = zone_to_nid(zone); > nr_pages = end_pfn - start_pfn; > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote: > Hi Wen, > > 2012/06/27 17:49, Wen Congyang wrote: >> At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote: >>> When offline_pages() is called to offlined memory, the function fails since >>> all memory has been offlined. In this case, the function should succeed. >>> The patch adds the check function into offline_pages(). >> >> You miss such case: some pages are online, while some pages are offline. >> offline_pages() will fail too in such case. > > You are right. But current code fails, when the function is called to offline > memory. In this case, the function should succeed. So the patch confirms > whether the memory was offlined or not. And if memory has already been > offlined, offline_pages return 0. Can you please explain why the caller can't check it? I hope to avoid an ignorance as far as we can.
Hi Kosaki-san, 2012/06/28 14:27, KOSAKI Motohiro wrote: > On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu > <isimatu.yasuaki@jp.fujitsu.com> wrote: >> Hi Wen, >> >> 2012/06/27 17:49, Wen Congyang wrote: >>> At 06/27/2012 01:44 PM, Yasuaki Ishimatsu Wrote: >>>> When offline_pages() is called to offlined memory, the function fails since >>>> all memory has been offlined. In this case, the function should succeed. >>>> The patch adds the check function into offline_pages(). >>> >>> You miss such case: some pages are online, while some pages are offline. >>> offline_pages() will fail too in such case. >> >> You are right. But current code fails, when the function is called to offline >> memory. In this case, the function should succeed. So the patch confirms >> whether the memory was offlined or not. And if memory has already been >> offlined, offline_pages return 0. > > Can you please explain why the caller can't check it? I hope to avoid > an ignorance > as far as we can. Of course, caller side can check it. But there is a possibility that offline_pages() is called by many functions. So I do not think that it is good that all functions which call offline_pages() check it. Thanks, Yasuaki Ishimatsu
Hi Kosaki-san, 2012/06/28 14:26, KOSAKI Motohiro wrote: > On Wed, Jun 27, 2012 at 1:44 AM, Yasuaki Ishimatsu > <isimatu.yasuaki@jp.fujitsu.com> wrote: >> When offline_pages() is called to offlined memory, the function fails since >> all memory has been offlined. In this case, the function should succeed. >> The patch adds the check function into offline_pages(). > > I don't understand your point. I think following misoperation should > fail. Otherwise > administrator have no way to know their fault. > > $ echo offline > memoryN/state > $ echo offline > memoryN/state > > In general, we don't like to ignore an error except the standard require it. I understood the intention of previous mail (why the caller can't check it? ). I'll move memory_is_offline() to caller side. >> >> CC: Len Brown <len.brown@intel.com> >> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> CC: Paul Mackerras <paulus@samba.org> >> CC: Christoph Lameter <cl@linux.com> >> Cc: Minchan Kim <minchan.kim@gmail.com> >> CC: Andrew Morton <akpm@linux-foundation.org> >> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> CC: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >> >> --- >> drivers/base/memory.c | 20 ++++++++++++++++++++ >> include/linux/memory.h | 1 + >> mm/memory_hotplug.c | 5 +++++ >> 3 files changed, 26 insertions(+) >> >> Index: linux-3.5-rc4/drivers/base/memory.c >> =================================================================== >> --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 >> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 >> @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( >> } >> EXPORT_SYMBOL(unregister_memory_isolate_notifier); >> >> +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) > > I dislike this function name. 'memory' is too vague to me. O.K. I retry to change the name of the function. > > >> +{ >> + struct memory_block *mem; >> + struct mem_section *section; >> + unsigned long pfn, section_nr; >> + >> + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >> + section_nr = pfn_to_section_nr(pfn); >> + section = __nr_to_section(section_nr); >> + mem = find_memory_block(section); > > This seems to have strong sparse dependency. Thanks. I will consider other CONFIG_. Thanks. Yasuaki Ishimatsu > Hm, I wonder why memory-hotplug.c can enable when X86_64_ACPI_NUMA. > > # eventually, we can have this option just 'select SPARSEMEM' > config MEMORY_HOTPLUG > bool "Allow for memory hot-add" > depends on SPARSEMEM || X86_64_ACPI_NUMA > > >> + if (!mem) >> + continue; >> + if (mem->state == MEM_OFFLINE) >> + continue; >> + return false; >> + } >> + >> + return true; >> +} >> + >> /* >> * register_memory - Setup a sysfs device for a memory block >> */ >> Index: linux-3.5-rc4/include/linux/memory.h >> =================================================================== >> --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 >> +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 >> @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne >> extern struct memory_block *find_memory_block_hinted(struct mem_section *, >> struct memory_block *); >> extern struct memory_block *find_memory_block(struct mem_section *); >> +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); >> #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) >> enum mem_add_context { BOOT, HOTPLUG }; >> #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> =================================================================== >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 >> @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned >> >> lock_memory_hotplug(); >> >> + if (memory_is_offline(start_pfn, end_pfn)) { >> + ret = 0; >> + goto out; >> + } >> + >> zone = page_zone(pfn_to_page(start_pfn)); >> node = zone_to_nid(zone); >> nr_pages = end_pfn - start_pfn; >> >> -- >> To unsubscribe, send a message with 'unsubscribe linux-mm' in >> the body to majordomo@kvack.org. For more info on Linux MM, >> see: http://www.linux-mm.org/ . >> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Hi David, 2012/06/27 15:16, David Rientjes wrote: > On Wed, 27 Jun 2012, Yasuaki Ishimatsu wrote: > >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> =================================================================== >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 >> @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned >> >> lock_memory_hotplug(); >> >> + if (memory_is_offline(start_pfn, end_pfn)) { >> + ret = 0; >> + goto out; >> + } >> + >> zone = page_zone(pfn_to_page(start_pfn)); >> node = zone_to_nid(zone); >> nr_pages = end_pfn - start_pfn; > > Are there additional prerequisites for this patch? Otherwise it changes > the return value of offline_memory() which will now call > acpi_memory_powerdown_device() in the acpi memhotplug case when disabling. > Is that a problem? I have understood there is a person who expects "offline_pages()" to fail in this case by kosaki's comment. So I'll move memory_is_offline to caller side. Thanks, Yasuaki Ishimatsu > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >
On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote: > When offline_pages() is called to offlined memory, the function fails since > all memory has been offlined. In this case, the function should succeed. > The patch adds the check function into offline_pages(). > > CC: Len Brown <len.brown@intel.com> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> > CC: Paul Mackerras <paulus@samba.org> > CC: Christoph Lameter <cl@linux.com> > Cc: Minchan Kim <minchan.kim@gmail.com> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > CC: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> > > --- > drivers/base/memory.c | 20 ++++++++++++++++++++ > include/linux/memory.h | 1 + > mm/memory_hotplug.c | 5 +++++ > 3 files changed, 26 insertions(+) > > Index: linux-3.5-rc4/drivers/base/memory.c > =================================================================== > --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 > +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 > @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( > } > EXPORT_SYMBOL(unregister_memory_isolate_notifier); > > +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) > +{ > + struct memory_block *mem; > + struct mem_section *section; > + unsigned long pfn, section_nr; > + > + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { > + section_nr = pfn_to_section_nr(pfn); > + section = __nr_to_section(section_nr); Is it possible for __nr_to_section return NULL here? > + mem = find_memory_block(section); > + if (!mem) > + continue; > + if (mem->state == MEM_OFFLINE) > + continue; > + return false; > + } > + > + return true; > +} Need a put_dev(&mem->dev) for the last memory block device handled before return. > + > /* > * register_memory - Setup a sysfs device for a memory block > */ > Index: linux-3.5-rc4/include/linux/memory.h > =================================================================== > --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 > +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 > @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne > extern struct memory_block *find_memory_block_hinted(struct mem_section *, > struct memory_block *); > extern struct memory_block *find_memory_block(struct mem_section *); > +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); > #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) > enum mem_add_context { BOOT, HOTPLUG }; > #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ > Index: linux-3.5-rc4/mm/memory_hotplug.c > =================================================================== > --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 > +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 > @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned > > lock_memory_hotplug(); > > + if (memory_is_offline(start_pfn, end_pfn)) { > + ret = 0; > + goto out; > + } > + > zone = page_zone(pfn_to_page(start_pfn)); > node = zone_to_nid(zone); > nr_pages = end_pfn - start_pfn; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote: > When offline_pages() is called to offlined memory, the function fails since > all memory has been offlined. In this case, the function should succeed. > The patch adds the check function into offline_pages(). > > CC: Len Brown <len.brown@intel.com> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> > CC: Paul Mackerras <paulus@samba.org> > CC: Christoph Lameter <cl@linux.com> > Cc: Minchan Kim <minchan.kim@gmail.com> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > CC: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> > > --- > drivers/base/memory.c | 20 ++++++++++++++++++++ > include/linux/memory.h | 1 + > mm/memory_hotplug.c | 5 +++++ > 3 files changed, 26 insertions(+) > > Index: linux-3.5-rc4/drivers/base/memory.c > =================================================================== > --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 > +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 > @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( > } > EXPORT_SYMBOL(unregister_memory_isolate_notifier); > > +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) > +{ > + struct memory_block *mem; > + struct mem_section *section; > + unsigned long pfn, section_nr; > + > + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { > + section_nr = pfn_to_section_nr(pfn); > + section = __nr_to_section(section_nr); > + mem = find_memory_block(section); Seems find_memory_block_hinted() is more efficient than find_memory_block() here. > + if (!mem) > + continue; > + if (mem->state == MEM_OFFLINE) > + continue; > + return false; > + } > + > + return true; > +} > + > /* > * register_memory - Setup a sysfs device for a memory block > */ > Index: linux-3.5-rc4/include/linux/memory.h > =================================================================== > --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 > +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 > @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne > extern struct memory_block *find_memory_block_hinted(struct mem_section *, > struct memory_block *); > extern struct memory_block *find_memory_block(struct mem_section *); > +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); > #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) > enum mem_add_context { BOOT, HOTPLUG }; > #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ > Index: linux-3.5-rc4/mm/memory_hotplug.c > =================================================================== > --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 > +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 > @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned > > lock_memory_hotplug(); > > + if (memory_is_offline(start_pfn, end_pfn)) { > + ret = 0; > + goto out; > + } > + > zone = page_zone(pfn_to_page(start_pfn)); > node = zone_to_nid(zone); > nr_pages = end_pfn - start_pfn; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi Jiang, Thank you for your feedback. 2012/07/01 0:46, Jiang Liu wrote: > On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote: >> When offline_pages() is called to offlined memory, the function fails since >> all memory has been offlined. In this case, the function should succeed. >> The patch adds the check function into offline_pages(). >> >> CC: Len Brown <len.brown@intel.com> >> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> CC: Paul Mackerras <paulus@samba.org> >> CC: Christoph Lameter <cl@linux.com> >> Cc: Minchan Kim <minchan.kim@gmail.com> >> CC: Andrew Morton <akpm@linux-foundation.org> >> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> CC: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >> >> --- >> drivers/base/memory.c | 20 ++++++++++++++++++++ >> include/linux/memory.h | 1 + >> mm/memory_hotplug.c | 5 +++++ >> 3 files changed, 26 insertions(+) >> >> Index: linux-3.5-rc4/drivers/base/memory.c >> =================================================================== >> --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 >> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 >> @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( >> } >> EXPORT_SYMBOL(unregister_memory_isolate_notifier); >> >> +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) >> +{ >> + struct memory_block *mem; >> + struct mem_section *section; >> + unsigned long pfn, section_nr; >> + >> + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >> + section_nr = pfn_to_section_nr(pfn); >> + section = __nr_to_section(section_nr); > Is it possible for __nr_to_section return NULL here? Yes. I'll add NULL check. > >> + mem = find_memory_block(section); >> + if (!mem) >> + continue; >> + if (mem->state == MEM_OFFLINE) >> + continue; >> + return false; >> + } >> + >> + return true; >> +} > Need a put_dev(&mem->dev) for the last memory block device handled before return. Thanks. I think kobject_put(&mem->dev.kobj) should be handled for all memory block devices found by find_memory_block(). I'll update it. Thanks, Yasuaki Ishimatsu > >> + >> /* >> * register_memory - Setup a sysfs device for a memory block >> */ >> Index: linux-3.5-rc4/include/linux/memory.h >> =================================================================== >> --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 >> +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 >> @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne >> extern struct memory_block *find_memory_block_hinted(struct mem_section *, >> struct memory_block *); >> extern struct memory_block *find_memory_block(struct mem_section *); >> +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); >> #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) >> enum mem_add_context { BOOT, HOTPLUG }; >> #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> =================================================================== >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 >> @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned >> >> lock_memory_hotplug(); >> >> + if (memory_is_offline(start_pfn, end_pfn)) { >> + ret = 0; >> + goto out; >> + } >> + >> zone = page_zone(pfn_to_page(start_pfn)); >> node = zone_to_nid(zone); >> nr_pages = end_pfn - start_pfn; >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >
Hi Jiang, 2012/07/01 0:51, Jiang Liu wrote: > On 06/27/2012 01:44 PM, Yasuaki Ishimatsu wrote: >> When offline_pages() is called to offlined memory, the function fails since >> all memory has been offlined. In this case, the function should succeed. >> The patch adds the check function into offline_pages(). >> >> CC: Len Brown <len.brown@intel.com> >> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> CC: Paul Mackerras <paulus@samba.org> >> CC: Christoph Lameter <cl@linux.com> >> Cc: Minchan Kim <minchan.kim@gmail.com> >> CC: Andrew Morton <akpm@linux-foundation.org> >> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> >> CC: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >> >> --- >> drivers/base/memory.c | 20 ++++++++++++++++++++ >> include/linux/memory.h | 1 + >> mm/memory_hotplug.c | 5 +++++ >> 3 files changed, 26 insertions(+) >> >> Index: linux-3.5-rc4/drivers/base/memory.c >> =================================================================== >> --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 >> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 >> @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( >> } >> EXPORT_SYMBOL(unregister_memory_isolate_notifier); >> >> +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) >> +{ >> + struct memory_block *mem; >> + struct mem_section *section; >> + unsigned long pfn, section_nr; >> + >> + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >> + section_nr = pfn_to_section_nr(pfn); >> + section = __nr_to_section(section_nr); >> + mem = find_memory_block(section); > Seems find_memory_block_hinted() is more efficient than find_memory_block() here. Thanks. I'll update it. Thanks, Yasuaki Ishimatsu > >> + if (!mem) >> + continue; >> + if (mem->state == MEM_OFFLINE) >> + continue; >> + return false; >> + } >> + >> + return true; >> +} >> + >> /* >> * register_memory - Setup a sysfs device for a memory block >> */ >> Index: linux-3.5-rc4/include/linux/memory.h >> =================================================================== >> --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 >> +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 >> @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne >> extern struct memory_block *find_memory_block_hinted(struct mem_section *, >> struct memory_block *); >> extern struct memory_block *find_memory_block(struct mem_section *); >> +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); >> #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) >> enum mem_add_context { BOOT, HOTPLUG }; >> #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> =================================================================== >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 >> @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned >> >> lock_memory_hotplug(); >> >> + if (memory_is_offline(start_pfn, end_pfn)) { >> + ret = 0; >> + goto out; >> + } >> + >> zone = page_zone(pfn_to_page(start_pfn)); >> node = zone_to_nid(zone); >> nr_pages = end_pfn - start_pfn; >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >
Index: linux-3.5-rc4/drivers/base/memory.c =================================================================== --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-26 13:28:16.726211752 +0900 +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:34:22.423639904 +0900 @@ -70,6 +70,26 @@ void unregister_memory_isolate_notifier( } EXPORT_SYMBOL(unregister_memory_isolate_notifier); +bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn) +{ + struct memory_block *mem; + struct mem_section *section; + unsigned long pfn, section_nr; + + for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) { + section_nr = pfn_to_section_nr(pfn); + section = __nr_to_section(section_nr); + mem = find_memory_block(section); + if (!mem) + continue; + if (mem->state == MEM_OFFLINE) + continue; + return false; + } + + return true; +} + /* * register_memory - Setup a sysfs device for a memory block */ Index: linux-3.5-rc4/include/linux/memory.h =================================================================== --- linux-3.5-rc4.orig/include/linux/memory.h 2012-06-25 04:53:04.000000000 +0900 +++ linux-3.5-rc4/include/linux/memory.h 2012-06-26 13:34:22.424639891 +0900 @@ -120,6 +120,7 @@ extern int memory_isolate_notify(unsigne extern struct memory_block *find_memory_block_hinted(struct mem_section *, struct memory_block *); extern struct memory_block *find_memory_block(struct mem_section *); +extern bool memory_is_offline(unsigned long start_pfn, unsigned long end_pfn); #define CONFIG_MEM_BLOCK_SIZE (PAGES_PER_SECTION<<PAGE_SHIFT) enum mem_add_context { BOOT, HOTPLUG }; #endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */ Index: linux-3.5-rc4/mm/memory_hotplug.c =================================================================== --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-26 13:28:16.743211538 +0900 +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:38.264940468 +0900 @@ -887,6 +887,11 @@ static int __ref offline_pages(unsigned lock_memory_hotplug(); + if (memory_is_offline(start_pfn, end_pfn)) { + ret = 0; + goto out; + } + zone = page_zone(pfn_to_page(start_pfn)); node = zone_to_nid(zone); nr_pages = end_pfn - start_pfn;
When offline_pages() is called to offlined memory, the function fails since all memory has been offlined. In this case, the function should succeed. The patch adds the check function into offline_pages(). CC: Len Brown <len.brown@intel.com> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org> CC: Paul Mackerras <paulus@samba.org> CC: Christoph Lameter <cl@linux.com> Cc: Minchan Kim <minchan.kim@gmail.com> CC: Andrew Morton <akpm@linux-foundation.org> CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> CC: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> --- drivers/base/memory.c | 20 ++++++++++++++++++++ include/linux/memory.h | 1 + mm/memory_hotplug.c | 5 +++++ 3 files changed, 26 insertions(+)