Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
Hi Wen, 2012/06/28 12:25, Wen Congyang wrote: > At 06/28/2012 11:01 AM, Yasuaki Ishimatsu Wrote: >> Hi David and Wen, >> >> Thank you for reviewing my patch. >> >> 2012/06/27 17:47, Wen Congyang wrote: >>> At 06/27/2012 03:14 PM, Wen Congyang Wrote: At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote: > remove_memory() does not remove memory but just offlines memory. The patch > changes name of it to offline_memory(). There are 3 functions in the kernel: 1. add_memory() 2. online_pages() 3. remove_memory() So I think offline_pages() is better than offline_memory(). >>> >>> There is already a function named offline_pages(). So we >>> should call offline_pages() instead of remove_memory() in >>> memory_block_action(), and there is no need to rename >>> remove_memory(). >> >> As Wen says, Linux has 4 functions for memory hotplug already. >> In my recognition, these functions are prepared for following purpose. >> >> 1. add_memory : add physical memory >> 2. online_pages : online logical memory >> 3. remove_memory : offline logical memory >> 4. offline_pages : offline logical memory >> >> add_memory() is used for adding physical memory. I think remove_memory() >> would rather be used for removing physical memory than be used for removing >> logical memory. So I renamed remove_memory() to offline_memory(). >> How do you think? > > Hmm, remove_memory() will revert all things we do in add_memory(), so I think I think so too. add_memory() prepares to use physical memory. It prepares some structures (pgdat, page table, node, etc) for using the physical memory at the system. But it does not online the meomory. For onlining the memory, we use online_pages(). So I think that remove_memory() should remove these structures which are prepared by add_memory() not offline memory. But current remove_memory() code only calls offline_pages() and offlines memory. The patch series recreates remove_memory() for removing these structures after [RFC PATCH 3/12]. The reason to change the name of remove_memory() is a preparation to recreate it. Thanks, Yasuaki Ishimatsu > there is no need to rename it. If we rename it to offline_memory(), we should > also rename add_memory() to online_memory(). > > Thanks > Wen Congyang > >> >> Regards, >> Yasuaki Ishimatsu >> >>> >>> Thanks >>> Wen Congyang >>> Thanks Wen Congyang > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
At 06/28/2012 11:01 AM, Yasuaki Ishimatsu Wrote: > Hi David and Wen, > > Thank you for reviewing my patch. > > 2012/06/27 17:47, Wen Congyang wrote: >> At 06/27/2012 03:14 PM, Wen Congyang Wrote: >>> At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote: remove_memory() does not remove memory but just offlines memory. The patch changes name of it to offline_memory(). >>> >>> There are 3 functions in the kernel: >>> 1. add_memory() >>> 2. online_pages() >>> 3. remove_memory() >>> >>> So I think offline_pages() is better than offline_memory(). >> >> There is already a function named offline_pages(). So we >> should call offline_pages() instead of remove_memory() in >> memory_block_action(), and there is no need to rename >> remove_memory(). > > As Wen says, Linux has 4 functions for memory hotplug already. > In my recognition, these functions are prepared for following purpose. > > 1. add_memory : add physical memory > 2. online_pages : online logical memory > 3. remove_memory : offline logical memory > 4. offline_pages : offline logical memory > > add_memory() is used for adding physical memory. I think remove_memory() > would rather be used for removing physical memory than be used for removing > logical memory. So I renamed remove_memory() to offline_memory(). > How do you think? Hmm, remove_memory() will revert all things we do in add_memory(), so I think there is no need to rename it. If we rename it to offline_memory(), we should also rename add_memory() to online_memory(). Thanks Wen Congyang > > Regards, > Yasuaki Ishimatsu > >> >> Thanks >> Wen Congyang >> >>> >>> Thanks >>> Wen Congyang ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
Hi David and Wen, Thank you for reviewing my patch. 2012/06/27 17:47, Wen Congyang wrote: > At 06/27/2012 03:14 PM, Wen Congyang Wrote: >> At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote: >>> remove_memory() does not remove memory but just offlines memory. The patch >>> changes name of it to offline_memory(). >> >> There are 3 functions in the kernel: >> 1. add_memory() >> 2. online_pages() >> 3. remove_memory() >> >> So I think offline_pages() is better than offline_memory(). > > There is already a function named offline_pages(). So we > should call offline_pages() instead of remove_memory() in > memory_block_action(), and there is no need to rename > remove_memory(). As Wen says, Linux has 4 functions for memory hotplug already. In my recognition, these functions are prepared for following purpose. 1. add_memory : add physical memory 2. online_pages : online logical memory 3. remove_memory : offline logical memory 4. offline_pages : offline logical memory add_memory() is used for adding physical memory. I think remove_memory() would rather be used for removing physical memory than be used for removing logical memory. So I renamed remove_memory() to offline_memory(). How do you think? Regards, Yasuaki Ishimatsu > > Thanks > Wen Congyang > >> >> Thanks >> Wen Congyang >>> >>> CC: Len Brown >>> CC: Benjamin Herrenschmidt >>> CC: Paul Mackerras >>> CC: Christoph Lameter >>> Cc: Minchan Kim >>> CC: Andrew Morton >>> CC: KOSAKI Motohiro >>> CC: Wen Congyang >>> Signed-off-by: Yasuaki Ishimatsu >>> >>> --- >>> drivers/acpi/acpi_memhotplug.c |2 +- >>> drivers/base/memory.c |4 ++-- >>> include/linux/memory_hotplug.h |2 +- >>> mm/memory_hotplug.c|6 +++--- >>> 4 files changed, 7 insertions(+), 7 deletions(-) >>> >>> Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c >>> === >>> --- linux-3.5-rc4.orig/drivers/acpi/acpi_memhotplug.c 2012-06-25 >>> 04:53:04.0 +0900 >>> +++ linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c2012-06-26 >>> 13:48:38.263940481 +0900 >>> @@ -318,7 +318,7 @@ static int acpi_memory_disable_device(st >>> */ >>> list_for_each_entry_safe(info, n, &mem_device->res_list, list) { >>> if (info->enabled) { >>> - result = remove_memory(info->start_addr, info->length); >>> + result = offline_memory(info->start_addr, info->length); >>> if (result) >>> return result; >>> } >>> Index: linux-3.5-rc4/drivers/base/memory.c >>> === >>> --- linux-3.5-rc4.orig/drivers/base/memory.c2012-06-25 >>> 04:53:04.0 +0900 >>> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:48:46.072842803 >>> +0900 >>> @@ -266,8 +266,8 @@ memory_block_action(unsigned long phys_i >>> break; >>> case MEM_OFFLINE: >>> start_paddr = page_to_pfn(first_page) << PAGE_SHIFT; >>> - ret = remove_memory(start_paddr, >>> - nr_pages << PAGE_SHIFT); >>> + ret = offline_memory(start_paddr, >>> +nr_pages << PAGE_SHIFT); >>> break; >>> default: >>> WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: " >>> Index: linux-3.5-rc4/mm/memory_hotplug.c >>> === >>> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-25 04:53:04.0 >>> +0900 >>> +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:46.072842803 >>> +0900 >>> @@ -990,7 +990,7 @@ out: >>> return ret; >>> } >>> >>> -int remove_memory(u64 start, u64 size) >>> +int offline_memory(u64 start, u64 size) >>> { >>> unsigned long start_pfn, end_pfn; >>> >>> @@ -999,9 +999,9 @@ int remove_memory(u64 start, u64 size) >>> return offline_pages(start_pfn, end_pfn, 120 * HZ); >>> } >>> #else >>> -int remove_memory(u64 start, u64 size) >>> +int offline_memory(u64 start, u64 size) >>> { >>> return -EINVAL; >>> } >>> #endif /* CONFIG_MEMORY_HOTREMOVE */ >>> -EXPORT_SYMBOL_GPL(remove_memory); >>> +EXPORT_SYMBOL_GPL(offline_memory); >>> Index: linux-3.5-rc4/include/linux/memory_hotplug.h >>> === >>> --- linux-3.5-rc4.orig/include/linux/memory_hotplug.h 2012-06-25 >>> 04:53:04.0 +0900 >>> +++ linux-3.5-rc4/include/linux/memory_hotplug.h2012-06-26 >>> 13:48:38.264940468 +0900 >>> @@ -233,7 +233,7 @@ static inline int is_mem_section_removab >>> extern int mem_online_node(int nid); >>> extern int add_memory(int nid, u64 start, u64 size); >>> extern int arch_add_memory(int nid, u64 start, u64 size); >>> -extern int remove_memory(u64 start, u64 size); >>> +ext
Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
At 06/27/2012 03:14 PM, Wen Congyang Wrote: > At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote: >> remove_memory() does not remove memory but just offlines memory. The patch >> changes name of it to offline_memory(). > > There are 3 functions in the kernel: > 1. add_memory() > 2. online_pages() > 3. remove_memory() > > So I think offline_pages() is better than offline_memory(). There is already a function named offline_pages(). So we should call offline_pages() instead of remove_memory() in memory_block_action(), and there is no need to rename remove_memory(). Thanks Wen Congyang > > Thanks > Wen Congyang >> >> CC: Len Brown >> CC: Benjamin Herrenschmidt >> CC: Paul Mackerras >> CC: Christoph Lameter >> Cc: Minchan Kim >> CC: Andrew Morton >> CC: KOSAKI Motohiro >> CC: Wen Congyang >> Signed-off-by: Yasuaki Ishimatsu >> >> --- >> drivers/acpi/acpi_memhotplug.c |2 +- >> drivers/base/memory.c |4 ++-- >> include/linux/memory_hotplug.h |2 +- >> mm/memory_hotplug.c|6 +++--- >> 4 files changed, 7 insertions(+), 7 deletions(-) >> >> Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c >> === >> --- linux-3.5-rc4.orig/drivers/acpi/acpi_memhotplug.c2012-06-25 >> 04:53:04.0 +0900 >> +++ linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c 2012-06-26 >> 13:48:38.263940481 +0900 >> @@ -318,7 +318,7 @@ static int acpi_memory_disable_device(st >> */ >> list_for_each_entry_safe(info, n, &mem_device->res_list, list) { >> if (info->enabled) { >> -result = remove_memory(info->start_addr, info->length); >> +result = offline_memory(info->start_addr, info->length); >> if (result) >> return result; >> } >> Index: linux-3.5-rc4/drivers/base/memory.c >> === >> --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-25 04:53:04.0 >> +0900 >> +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:48:46.072842803 >> +0900 >> @@ -266,8 +266,8 @@ memory_block_action(unsigned long phys_i >> break; >> case MEM_OFFLINE: >> start_paddr = page_to_pfn(first_page) << PAGE_SHIFT; >> -ret = remove_memory(start_paddr, >> -nr_pages << PAGE_SHIFT); >> +ret = offline_memory(start_paddr, >> + nr_pages << PAGE_SHIFT); >> break; >> default: >> WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: " >> Index: linux-3.5-rc4/mm/memory_hotplug.c >> === >> --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-25 04:53:04.0 >> +0900 >> +++ linux-3.5-rc4/mm/memory_hotplug.c2012-06-26 13:48:46.072842803 >> +0900 >> @@ -990,7 +990,7 @@ out: >> return ret; >> } >> >> -int remove_memory(u64 start, u64 size) >> +int offline_memory(u64 start, u64 size) >> { >> unsigned long start_pfn, end_pfn; >> >> @@ -999,9 +999,9 @@ int remove_memory(u64 start, u64 size) >> return offline_pages(start_pfn, end_pfn, 120 * HZ); >> } >> #else >> -int remove_memory(u64 start, u64 size) >> +int offline_memory(u64 start, u64 size) >> { >> return -EINVAL; >> } >> #endif /* CONFIG_MEMORY_HOTREMOVE */ >> -EXPORT_SYMBOL_GPL(remove_memory); >> +EXPORT_SYMBOL_GPL(offline_memory); >> Index: linux-3.5-rc4/include/linux/memory_hotplug.h >> === >> --- linux-3.5-rc4.orig/include/linux/memory_hotplug.h2012-06-25 >> 04:53:04.0 +0900 >> +++ linux-3.5-rc4/include/linux/memory_hotplug.h 2012-06-26 >> 13:48:38.264940468 +0900 >> @@ -233,7 +233,7 @@ static inline int is_mem_section_removab >> extern int mem_online_node(int nid); >> extern int add_memory(int nid, u64 start, u64 size); >> extern int arch_add_memory(int nid, u64 start, u64 size); >> -extern int remove_memory(u64 start, u64 size); >> +extern int offline_memory(u64 start, u64 size); >> extern int sparse_add_one_section(struct zone *zone, unsigned long >> start_pfn, >> int nr_pages); >> extern void sparse_remove_one_section(struct zone *zone, struct mem_section >> *ms); >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-in
Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
At 06/27/2012 01:42 PM, Yasuaki Ishimatsu Wrote: > remove_memory() does not remove memory but just offlines memory. The patch > changes name of it to offline_memory(). There are 3 functions in the kernel: 1. add_memory() 2. online_pages() 3. remove_memory() So I think offline_pages() is better than offline_memory(). Thanks Wen Congyang > > CC: Len Brown > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Christoph Lameter > Cc: Minchan Kim > CC: Andrew Morton > CC: KOSAKI Motohiro > CC: Wen Congyang > Signed-off-by: Yasuaki Ishimatsu > > --- > drivers/acpi/acpi_memhotplug.c |2 +- > drivers/base/memory.c |4 ++-- > include/linux/memory_hotplug.h |2 +- > mm/memory_hotplug.c|6 +++--- > 4 files changed, 7 insertions(+), 7 deletions(-) > > Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c > === > --- linux-3.5-rc4.orig/drivers/acpi/acpi_memhotplug.c 2012-06-25 > 04:53:04.0 +0900 > +++ linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c 2012-06-26 > 13:48:38.263940481 +0900 > @@ -318,7 +318,7 @@ static int acpi_memory_disable_device(st >*/ > list_for_each_entry_safe(info, n, &mem_device->res_list, list) { > if (info->enabled) { > - result = remove_memory(info->start_addr, info->length); > + result = offline_memory(info->start_addr, info->length); > if (result) > return result; > } > Index: linux-3.5-rc4/drivers/base/memory.c > === > --- linux-3.5-rc4.orig/drivers/base/memory.c 2012-06-25 04:53:04.0 > +0900 > +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:48:46.072842803 > +0900 > @@ -266,8 +266,8 @@ memory_block_action(unsigned long phys_i > break; > case MEM_OFFLINE: > start_paddr = page_to_pfn(first_page) << PAGE_SHIFT; > - ret = remove_memory(start_paddr, > - nr_pages << PAGE_SHIFT); > + ret = offline_memory(start_paddr, > + nr_pages << PAGE_SHIFT); > break; > default: > WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: " > Index: linux-3.5-rc4/mm/memory_hotplug.c > === > --- linux-3.5-rc4.orig/mm/memory_hotplug.c2012-06-25 04:53:04.0 > +0900 > +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:46.072842803 +0900 > @@ -990,7 +990,7 @@ out: > return ret; > } > > -int remove_memory(u64 start, u64 size) > +int offline_memory(u64 start, u64 size) > { > unsigned long start_pfn, end_pfn; > > @@ -999,9 +999,9 @@ int remove_memory(u64 start, u64 size) > return offline_pages(start_pfn, end_pfn, 120 * HZ); > } > #else > -int remove_memory(u64 start, u64 size) > +int offline_memory(u64 start, u64 size) > { > return -EINVAL; > } > #endif /* CONFIG_MEMORY_HOTREMOVE */ > -EXPORT_SYMBOL_GPL(remove_memory); > +EXPORT_SYMBOL_GPL(offline_memory); > Index: linux-3.5-rc4/include/linux/memory_hotplug.h > === > --- linux-3.5-rc4.orig/include/linux/memory_hotplug.h 2012-06-25 > 04:53:04.0 +0900 > +++ linux-3.5-rc4/include/linux/memory_hotplug.h 2012-06-26 > 13:48:38.264940468 +0900 > @@ -233,7 +233,7 @@ static inline int is_mem_section_removab > extern int mem_online_node(int nid); > extern int add_memory(int nid, u64 start, u64 size); > extern int arch_add_memory(int nid, u64 start, u64 size); > -extern int remove_memory(u64 start, u64 size); > +extern int offline_memory(u64 start, u64 size); > extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn, > int nr_pages); > extern void sparse_remove_one_section(struct zone *zone, struct mem_section > *ms); > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
remove_memory() does not remove memory but just offlines memory. The patch changes name of it to offline_memory(). CC: Len Brown CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Christoph Lameter Cc: Minchan Kim CC: Andrew Morton CC: KOSAKI Motohiro CC: Wen Congyang Signed-off-by: Yasuaki Ishimatsu --- drivers/acpi/acpi_memhotplug.c |2 +- drivers/base/memory.c |4 ++-- include/linux/memory_hotplug.h |2 +- mm/memory_hotplug.c|6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) Index: linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c === --- linux-3.5-rc4.orig/drivers/acpi/acpi_memhotplug.c 2012-06-25 04:53:04.0 +0900 +++ linux-3.5-rc4/drivers/acpi/acpi_memhotplug.c2012-06-26 13:48:38.263940481 +0900 @@ -318,7 +318,7 @@ static int acpi_memory_disable_device(st */ list_for_each_entry_safe(info, n, &mem_device->res_list, list) { if (info->enabled) { - result = remove_memory(info->start_addr, info->length); + result = offline_memory(info->start_addr, info->length); if (result) return result; } Index: linux-3.5-rc4/drivers/base/memory.c === --- linux-3.5-rc4.orig/drivers/base/memory.c2012-06-25 04:53:04.0 +0900 +++ linux-3.5-rc4/drivers/base/memory.c 2012-06-26 13:48:46.072842803 +0900 @@ -266,8 +266,8 @@ memory_block_action(unsigned long phys_i break; case MEM_OFFLINE: start_paddr = page_to_pfn(first_page) << PAGE_SHIFT; - ret = remove_memory(start_paddr, - nr_pages << PAGE_SHIFT); + ret = offline_memory(start_paddr, +nr_pages << PAGE_SHIFT); break; default: WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: " Index: linux-3.5-rc4/mm/memory_hotplug.c === --- linux-3.5-rc4.orig/mm/memory_hotplug.c 2012-06-25 04:53:04.0 +0900 +++ linux-3.5-rc4/mm/memory_hotplug.c 2012-06-26 13:48:46.072842803 +0900 @@ -990,7 +990,7 @@ out: return ret; } -int remove_memory(u64 start, u64 size) +int offline_memory(u64 start, u64 size) { unsigned long start_pfn, end_pfn; @@ -999,9 +999,9 @@ int remove_memory(u64 start, u64 size) return offline_pages(start_pfn, end_pfn, 120 * HZ); } #else -int remove_memory(u64 start, u64 size) +int offline_memory(u64 start, u64 size) { return -EINVAL; } #endif /* CONFIG_MEMORY_HOTREMOVE */ -EXPORT_SYMBOL_GPL(remove_memory); +EXPORT_SYMBOL_GPL(offline_memory); Index: linux-3.5-rc4/include/linux/memory_hotplug.h === --- linux-3.5-rc4.orig/include/linux/memory_hotplug.h 2012-06-25 04:53:04.0 +0900 +++ linux-3.5-rc4/include/linux/memory_hotplug.h2012-06-26 13:48:38.264940468 +0900 @@ -233,7 +233,7 @@ static inline int is_mem_section_removab extern int mem_online_node(int nid); extern int add_memory(int nid, u64 start, u64 size); extern int arch_add_memory(int nid, u64 start, u64 size); -extern int remove_memory(u64 start, u64 size); +extern int offline_memory(u64 start, u64 size); extern int sparse_add_one_section(struct zone *zone, unsigned long start_pfn, int nr_pages); extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms); ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH 1/12] memory-hotplug : rename remove_memory to offline_memory
On Wed, 27 Jun 2012, Yasuaki Ishimatsu wrote: > remove_memory() does not remove memory but just offlines memory. The patch > changes name of it to offline_memory(). > The kernel is never going to physically remove the memory itself, so I don't see the big problem with calling it remove_memory(). If you're going to change it to offline_memory(), which is just as good but not better, then I'd suggest changing add_memory() to online_memory() for completeness. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev