Re: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
At 10/19/2012 08:49 AM, Yasuaki Ishimatsu Wrote: > Hi Kosaki, > > Sorry for late reply. > > 2012/10/13 4:28, KOSAKI Motohiro wrote: >> On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu >> wrote: >>> The function get_page_bootmem() may be called more than one time to >>> the same >>> page. There is no need to set page's type, private if the function is >>> not >>> the first time called to the page. >>> >>> Note: the patch is just optimization and does not fix any problem. >>> >>> CC: David Rientjes >>> CC: Jiang Liu >>> CC: Len Brown >>> 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 file changed, 11 insertions(+), 4 deletions(-) >>> >>> Index: linux-3.6/mm/memory_hotplug.c >>> === >>> --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 >>> +0900 >>> +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 >>> +0900 >>> @@ -95,10 +95,17 @@ static void release_memory_resource(stru >>> 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 I understand correctly, page->lru.next might be uninitialized yet. > > Ah yes. I was misunderstanding... > > Hi Wen, > > When you update the physical hot remove patch-set, please drop the patch. OK Thanks Wen Congyang > > Thanks, > Yasuaki Ishimatsu >> Moreover, I have no seen any good effect in this patch. I don't >> understand >> why we need to increase code complexity. >> >> >> >>> + 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 >>> >>> -- >>> 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: mailto:"d...@kvack.org;> em...@kvack.org >> -- >> 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-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-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
Hi Kosaki, Sorry for late reply. 2012/10/13 4:28, KOSAKI Motohiro wrote: On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu wrote: The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes CC: Jiang Liu CC: Len Brown 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 I understand correctly, page->lru.next might be uninitialized yet. Ah yes. I was misunderstanding... Hi Wen, When you update the physical hot remove patch-set, please drop the patch. Thanks, Yasuaki Ishimatsu Moreover, I have no seen any good effect in this patch. I don't understand why we need to increase code complexity. + 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 -- 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: mailto:"d...@kvack.org;> em...@kvack.org -- 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-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
Hi Kosaki, Sorry for late reply. 2012/10/13 4:28, KOSAKI Motohiro wrote: On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes rient...@google.com CC: Jiang Liu liu...@gmail.com CC: Len Brown len.br...@intel.com 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 I understand correctly, page-lru.next might be uninitialized yet. Ah yes. I was misunderstanding... Hi Wen, When you update the physical hot remove patch-set, please drop the patch. Thanks, Yasuaki Ishimatsu Moreover, I have no seen any good effect in this patch. I don't understand why we need to increase code complexity. + 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 -- 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 -- 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-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
At 10/19/2012 08:49 AM, Yasuaki Ishimatsu Wrote: Hi Kosaki, Sorry for late reply. 2012/10/13 4:28, KOSAKI Motohiro wrote: On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes rient...@google.com CC: Jiang Liu liu...@gmail.com CC: Len Brown len.br...@intel.com 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 I understand correctly, page-lru.next might be uninitialized yet. Ah yes. I was misunderstanding... Hi Wen, When you update the physical hot remove patch-set, please drop the patch. OK Thanks Wen Congyang Thanks, Yasuaki Ishimatsu Moreover, I have no seen any good effect in this patch. I don't understand why we need to increase code complexity. + 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 -- 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 -- 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-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-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu wrote: > The function get_page_bootmem() may be called more than one time to the same > page. There is no need to set page's type, private if the function is not > the first time called to the page. > > Note: the patch is just optimization and does not fix any problem. > > CC: David Rientjes > CC: Jiang Liu > CC: Len Brown > 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 file changed, 11 insertions(+), 4 deletions(-) > > Index: linux-3.6/mm/memory_hotplug.c > === > --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 > +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 > @@ -95,10 +95,17 @@ static void release_memory_resource(stru > 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 I understand correctly, page->lru.next might be uninitialized yet. Moreover, I have no seen any good effect in this patch. I don't understand why we need to increase code complexity. > + 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 > > -- > 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: mailto:"d...@kvack.org;> em...@kvack.org -- 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: [PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
On Thu, Oct 4, 2012 at 10:32 PM, Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com wrote: The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes rient...@google.com CC: Jiang Liu liu...@gmail.com CC: Len Brown len.br...@intel.com 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 I understand correctly, page-lru.next might be uninitialized yet. Moreover, I have no seen any good effect in this patch. I don't understand why we need to increase code complexity. + 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 -- 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 -- 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/
[PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes CC: Jiang Liu CC: Len Brown 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 -- 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/
[PATCH 5/10] memory-hotplug : memory-hotplug: check page type in get_page_bootmem
The function get_page_bootmem() may be called more than one time to the same page. There is no need to set page's type, private if the function is not the first time called to the page. Note: the patch is just optimization and does not fix any problem. CC: David Rientjes rient...@google.com CC: Jiang Liu liu...@gmail.com CC: Len Brown len.br...@intel.com 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 file changed, 11 insertions(+), 4 deletions(-) Index: linux-3.6/mm/memory_hotplug.c === --- linux-3.6.orig/mm/memory_hotplug.c 2012-10-04 18:29:58.284676075 +0900 +++ linux-3.6/mm/memory_hotplug.c 2012-10-04 18:30:03.454680542 +0900 @@ -95,10 +95,17 @@ static void release_memory_resource(stru 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 -- 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/