Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Yasuaki Ishimatsu]

Hi Wen,

2012/09/04 12:46, Wen Congyang wrote:

Hi, isimatu-san

At 09/01/2012 05:30 AM, Andrew Morton Wrote:

On Tue, 28 Aug 2012 18:00:20 +0800
we...@cn.fujitsu.com wrote:


From: Yasuaki Ishimatsu 

There is a possibility that get_page_bootmem() is called to the same page many
times. So when get_page_bootmem is called to the same page, the function only
increments page->_count.


I really don't understand this explanation, even after having looked at
the code.  Can you please have another attempt at the changelog?


What is the problem that you want to fix? The function get_page_bootmem()
may be called to the same page more than once, but I don't find any problem
about current implementation.


The patch is just optimization. The patch does not fix a problems.
As you know, the function may be called many times for the same page.
I think if a page is sets to page_type and Page Private flag and page->private,
the page need not be set the same things again. So we check page_type when
get_page_bootmem() is called. And if the page has been set to them, the page
is only incremented page->_count.

Thanks,
Yasuaki Ishimatsu



Thanks
Wen Congyang




--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
  static void get_page_bootmem(unsigned long info,  struct page *page,
 unsigned long type)
  {
-   page->lru.next = (struct list_head *) type;
-   SetPagePrivate(page);
-   set_page_private(page, info);
-   atomic_inc(>_count);
+   unsigned long page_type;
+
+   page_type = (unsigned long) page->lru.next;
+   if (page_type < MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
+   page_type > MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
+   page->lru.next = (struct list_head *) type;
+   SetPagePrivate(page);
+   set_page_private(page, info);
+   atomic_inc(>_count);
+   } else
+   atomic_inc(>_count);
  }


And a code comment which explains what is going on would be good.  As
is always the case ;)







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

Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Yasuaki Ishimatsu]

Hi Wen,

2012/09/04 12:46, Wen Congyang wrote:

Hi, isimatu-san

At 09/01/2012 05:30 AM, Andrew Morton Wrote:

On Tue, 28 Aug 2012 18:00:20 +0800
we...@cn.fujitsu.com wrote:


From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com

There is a possibility that get_page_bootmem() is called to the same page many
times. So when get_page_bootmem is called to the same page, the function only
increments page-_count.


I really don't understand this explanation, even after having looked at
the code.  Can you please have another attempt at the changelog?


What is the problem that you want to fix? The function get_page_bootmem()
may be called to the same page more than once, but I don't find any problem
about current implementation.


The patch is just optimization. The patch does not fix a problems.
As you know, the function may be called many times for the same page.
I think if a page is sets to page_type and Page Private flag and page-private,
the page need not be set the same things again. So we check page_type when
get_page_bootmem() is called. And if the page has been set to them, the page
is only incremented page-_count.

Thanks,
Yasuaki Ishimatsu



Thanks
Wen Congyang




--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
  static void get_page_bootmem(unsigned long info,  struct page *page,
 unsigned long type)
  {
-   page-lru.next = (struct list_head *) type;
-   SetPagePrivate(page);
-   set_page_private(page, info);
-   atomic_inc(page-_count);
+   unsigned long page_type;
+
+   page_type = (unsigned long) page-lru.next;
+   if (page_type  MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
+   page_type  MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
+   page-lru.next = (struct list_head *) type;
+   SetPagePrivate(page);
+   set_page_private(page, info);
+   atomic_inc(page-_count);
+   } else
+   atomic_inc(page-_count);
  }


And a code comment which explains what is going on would be good.  As
is always the case ;)







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

Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Wen Congyang]

Hi, isimatu-san

At 09/01/2012 05:30 AM, Andrew Morton Wrote:
> On Tue, 28 Aug 2012 18:00:20 +0800
> we...@cn.fujitsu.com wrote:
> 
>> From: Yasuaki Ishimatsu 
>>
>> There is a possibility that get_page_bootmem() is called to the same page 
>> many
>> times. So when get_page_bootmem is called to the same page, the function only
>> increments page->_count.
> 
> I really don't understand this explanation, even after having looked at
> the code.  Can you please have another attempt at the changelog?

What is the problem that you want to fix? The function get_page_bootmem()
may be called to the same page more than once, but I don't find any problem
about current implementation.

Thanks
Wen Congyang

> 
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
>>  static void get_page_bootmem(unsigned long info,  struct page *page,
>>   unsigned long type)
>>  {
>> -page->lru.next = (struct list_head *) type;
>> -SetPagePrivate(page);
>> -set_page_private(page, info);
>> -atomic_inc(>_count);
>> +unsigned long page_type;
>> +
>> +page_type = (unsigned long) page->lru.next;
>> +if (page_type < MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
>> +page_type > MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
>> +page->lru.next = (struct list_head *) type;
>> +SetPagePrivate(page);
>> +set_page_private(page, info);
>> +atomic_inc(>_count);
>> +} else
>> +atomic_inc(>_count);
>>  }
> 
> And a code comment which explains what is going on would be good.  As
> is always the case ;)
> 
> 

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

Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Wen Congyang]

Hi, isimatu-san

At 09/01/2012 05:30 AM, Andrew Morton Wrote:
 On Tue, 28 Aug 2012 18:00:20 +0800
 we...@cn.fujitsu.com wrote:
 
 From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com

 There is a possibility that get_page_bootmem() is called to the same page 
 many
 times. So when get_page_bootmem is called to the same page, the function only
 increments page-_count.
 
 I really don't understand this explanation, even after having looked at
 the code.  Can you please have another attempt at the changelog?

What is the problem that you want to fix? The function get_page_bootmem()
may be called to the same page more than once, but I don't find any problem
about current implementation.

Thanks
Wen Congyang

 
 --- a/mm/memory_hotplug.c
 +++ b/mm/memory_hotplug.c
 @@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
  static void get_page_bootmem(unsigned long info,  struct page *page,
   unsigned long type)
  {
 -page-lru.next = (struct list_head *) type;
 -SetPagePrivate(page);
 -set_page_private(page, info);
 -atomic_inc(page-_count);
 +unsigned long page_type;
 +
 +page_type = (unsigned long) page-lru.next;
 +if (page_type  MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
 +page_type  MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
 +page-lru.next = (struct list_head *) type;
 +SetPagePrivate(page);
 +set_page_private(page, info);
 +atomic_inc(page-_count);
 +} else
 +atomic_inc(page-_count);
  }
 
 And a code comment which explains what is going on would be good.  As
 is always the case ;)
 
 

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

Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Andrew Morton]

On Tue, 28 Aug 2012 18:00:20 +0800
we...@cn.fujitsu.com wrote:

> From: Yasuaki Ishimatsu 
> 
> There is a possibility that get_page_bootmem() is called to the same page many
> times. So when get_page_bootmem is called to the same page, the function only
> increments page->_count.

I really don't understand this explanation, even after having looked at
the code.  Can you please have another attempt at the changelog?

> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
>  static void get_page_bootmem(unsigned long info,  struct page *page,
>unsigned long type)
>  {
> - page->lru.next = (struct list_head *) type;
> - SetPagePrivate(page);
> - set_page_private(page, info);
> - atomic_inc(>_count);
> + unsigned long page_type;
> +
> + page_type = (unsigned long) page->lru.next;
> + if (page_type < MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
> + page_type > MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
> + page->lru.next = (struct list_head *) type;
> + SetPagePrivate(page);
> + set_page_private(page, info);
> + atomic_inc(>_count);
> + } else
> + atomic_inc(>_count);
>  }

And a code comment which explains what is going on would be good.  As
is always the case ;)

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

Re: [RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[Andrew Morton]

On Tue, 28 Aug 2012 18:00:20 +0800
we...@cn.fujitsu.com wrote:

 From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
 
 There is a possibility that get_page_bootmem() is called to the same page many
 times. So when get_page_bootmem is called to the same page, the function only
 increments page-_count.

I really don't understand this explanation, even after having looked at
the code.  Can you please have another attempt at the changelog?

 --- a/mm/memory_hotplug.c
 +++ b/mm/memory_hotplug.c
 @@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
  static void get_page_bootmem(unsigned long info,  struct page *page,
unsigned long type)
  {
 - page-lru.next = (struct list_head *) type;
 - SetPagePrivate(page);
 - set_page_private(page, info);
 - atomic_inc(page-_count);
 + unsigned long page_type;
 +
 + page_type = (unsigned long) page-lru.next;
 + if (page_type  MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
 + page_type  MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
 + page-lru.next = (struct list_head *) type;
 + SetPagePrivate(page);
 + set_page_private(page, info);
 + atomic_inc(page-_count);
 + } else
 + atomic_inc(page-_count);
  }

And a code comment which explains what is going on would be good.  As
is always the case ;)

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

[RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[wency]

From: Yasuaki Ishimatsu 

There is a possibility that get_page_bootmem() is called to the same page many
times. So when get_page_bootmem is called to the same page, the function only
increments page->_count.

CC: David Rientjes 
CC: Jiang Liu 
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 
---
 mm/memory_hotplug.c |   15 +++
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 5f9f8c7..d85af6d 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
 static void get_page_bootmem(unsigned long info,  struct page *page,
 unsigned long type)
 {
-   page->lru.next = (struct list_head *) type;
-   SetPagePrivate(page);
-   set_page_private(page, info);
-   atomic_inc(>_count);
+   unsigned long page_type;
+
+   page_type = (unsigned long) page->lru.next;
+   if (page_type < MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
+   page_type > MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
+   page->lru.next = (struct list_head *) type;
+   SetPagePrivate(page);
+   set_page_private(page, info);
+   atomic_inc(>_count);
+   } else
+   atomic_inc(>_count);
 }
 
 /* reference to __meminit __free_pages_bootmem is valid
-- 
1.7.1

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

[RFC v8 PATCH 13/20] memory-hotplug: check page type in get_page_bootmem

[wency]

From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com

There is a possibility that get_page_bootmem() is called to the same page many
times. So when get_page_bootmem is called to the same page, the function only
increments page-_count.

CC: David Rientjes rient...@google.com
CC: Jiang Liu liu...@gmail.com
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
---
 mm/memory_hotplug.c |   15 +++
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 5f9f8c7..d85af6d 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -95,10 +95,17 @@ static void release_memory_resource(struct resource *res)
 static void get_page_bootmem(unsigned long info,  struct page *page,
 unsigned long type)
 {
-   page-lru.next = (struct list_head *) type;
-   SetPagePrivate(page);
-   set_page_private(page, info);
-   atomic_inc(page-_count);
+   unsigned long page_type;
+
+   page_type = (unsigned long) page-lru.next;
+   if (page_type  MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE ||
+   page_type  MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE){
+   page-lru.next = (struct list_head *) type;
+   SetPagePrivate(page);
+   set_page_private(page, info);
+   atomic_inc(page-_count);
+   } else
+   atomic_inc(page-_count);
 }
 
 /* reference to __meminit __free_pages_bootmem is valid
-- 
1.7.1

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