Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-07-01 Thread Yasuaki Ishimatsu 
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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.h2012-06-25 
 04:53:04.0 +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_SECTIONPAGE_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.c2012-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 majord...@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 majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
 



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-07-01 Thread Yasuaki Ishimatsu 
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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.h2012-06-25 
 04:53:04.0 +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_SECTIONPAGE_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.c2012-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 majord...@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 majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
 



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-30 Thread Jiang Liu 
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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.0 
 +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_SECTIONPAGE_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.c2012-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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-30 Thread Jiang Liu 
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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.0 
 +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_SECTIONPAGE_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.c2012-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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-28 Thread Yasuaki Ishimatsu 

Hi Kosaki-san,

2012/06/28 14:27, KOSAKI Motohiro wrote:

On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu
isimatu.yasu...@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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-28 Thread 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.yasu...@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.br...@intel.com
CC: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: Paul Mackerras pau...@samba.org
CC: Christoph Lameter c...@linux.com
Cc: Minchan Kim minchan@gmail.com
CC: Andrew Morton a...@linux-foundation.org
CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
CC: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.c2012-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.0 
+0900
+++ linux-3.5-rc4/include/linux/memory.h2012-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_SECTIONPAGE_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 majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a




___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-28 Thread Yasuaki Ishimatsu 

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 majord...@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a





___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-27 Thread David Rientjes 
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.c2012-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?
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-27 Thread Yasuaki Ishimatsu 
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.br...@intel.com
CC: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: Paul Mackerras pau...@samba.org
CC: Christoph Lameter c...@linux.com
Cc: Minchan Kim minchan@gmail.com
CC: Andrew Morton a...@linux-foundation.org
CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
CC: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.c2012-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.0 
+0900
+++ linux-3.5-rc4/include/linux/memory.h2012-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_SECTIONPAGE_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;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-27 Thread Wen Congyang 
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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.0 
 +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_SECTIONPAGE_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.c2012-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 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

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-27 Thread KOSAKI Motohiro 
On Wed, Jun 27, 2012 at 1:44 AM, Yasuaki Ishimatsu
isimatu.yasu...@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.br...@intel.com
 CC: Benjamin Herrenschmidt b...@kernel.crashing.org
 CC: Paul Mackerras pau...@samba.org
 CC: Christoph Lameter c...@linux.com
 Cc: Minchan Kim minchan@gmail.com
 CC: Andrew Morton a...@linux-foundation.org
 CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
 CC: Wen Congyang we...@cn.fujitsu.com
 Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@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.0 
 +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_SECTIONPAGE_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 majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 2/12] memory-hogplug : check memory offline in offline_pages

2012-06-27 Thread KOSAKI Motohiro 
On Thu, Jun 28, 2012 at 1:06 AM, Yasuaki Ishimatsu
isimatu.yasu...@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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev