subject:"\[PATCH 2\/3\] mm\: remain migratetype in freed page"

Re: [PATCH 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Minchan Kim

On Wed, Sep 05, 2012 at 10:25:34AM +0100, Mel Gorman wrote:
> On Wed, Sep 05, 2012 at 04:26:01PM +0900, Minchan Kim wrote:
> > Page allocator doesn't keep migratetype information to page
> > when the page is freed. This patch remains the information
> > to freed page's index field which isn't used by free/alloc
> > preparing so it shouldn't change any behavir except below one.
> > 
> 
> This explanation could have been a *LOT* more helpful.
> 
> The page allocator caches the pageblock information in page->private while
> it is in the PCP freelists but this is overwritten with the order of the
> page when freed to the buddy allocator. This patch stores the migratetype
> of the page in the page->index field so that it is available at all times.

I will add your comment in my description.

> 
> > This patch adds a new call site in __free_pages_ok so it might be
> > overhead a bit but it's for high order allocation.
> > So I believe damage isn't hurt.
> > 
> 
> The additional call to set_page_migratetype() is not heavy. If you were
> adding a new call to get_pageblock_migratetype() or something equally
> expensive I would be more concerned.

I'm lucky to avoid your keen eye. ;)

> 
> > Signed-off-by: Minchan Kim 
> 
> The information you store in the page->index becomes stale if the page
> gets moved to another free list by move_freepages(). Not sure if that is
> a problem for you or not but it is possible that
> get_page_migratetype(page) != get_pageblock_migratetype(page)

Thanks for the spot. I have to fix it.

> 
> > ---
> >  include/linux/mm.h |6 --
> >  mm/page_alloc.c|7 ---
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 86d61d6..8fd32da 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -251,12 +251,14 @@ struct inode;
> >  
> >  static inline void set_page_migratetype(struct page *page, int migratetype)
> >  {
> > -   set_page_private(page, migratetype);
> > +   VM_BUG_ON((unsigned int)migratetype >= MIGRATE_TYPES);
> 
> This additional bug check is not mentioned in the changelog. Not clear
> if it's necessary.

I'm not strong so if anyone think it's not necessary, I will drop.

> 
> > +   page->index = migratetype;
> >  }
> >  
> >  static inline int get_page_migratetype(struct page *page)
> >  {
> > -   return page_private(page);
> > +   VM_BUG_ON((unsigned int)page->index >= MIGRATE_TYPES);
> > +   return page->index;
> >  }
> >  
> >  /*
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 103ba66..32985dd 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned 
> > int order)
> >  {
> > unsigned long flags;
> > int wasMlocked = __TestClearPageMlocked(page);
> > +   int migratetype;
> >  
> > if (!free_pages_prepare(page, order))
> > return;
> > @@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned 
> > int order)
> > if (unlikely(wasMlocked))
> > free_page_mlock(page);
> > __count_vm_events(PGFREE, 1 << order);
> > -   free_one_page(page_zone(page), page, order,
> > -   get_pageblock_migratetype(page));
> > -
> > +   migratetype = get_pageblock_migratetype(page);
> > +   set_page_migratetype(page, migratetype);
> > +   free_one_page(page_zone(page), page, order, migratetype);
> > local_irq_restore(flags);
> >  }
> >  
> 
> -- 
> Mel Gorman
> SUSE Labs
> 
> --
> 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 

-- 
Kind regards,
Minchan Kim
--
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 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Mel Gorman

On Wed, Sep 05, 2012 at 04:26:01PM +0900, Minchan Kim wrote:
> Page allocator doesn't keep migratetype information to page
> when the page is freed. This patch remains the information
> to freed page's index field which isn't used by free/alloc
> preparing so it shouldn't change any behavir except below one.
> 

This explanation could have been a *LOT* more helpful.

The page allocator caches the pageblock information in page->private while
it is in the PCP freelists but this is overwritten with the order of the
page when freed to the buddy allocator. This patch stores the migratetype
of the page in the page->index field so that it is available at all times.

> This patch adds a new call site in __free_pages_ok so it might be
> overhead a bit but it's for high order allocation.
> So I believe damage isn't hurt.
> 

The additional call to set_page_migratetype() is not heavy. If you were
adding a new call to get_pageblock_migratetype() or something equally
expensive I would be more concerned.

> Signed-off-by: Minchan Kim 

The information you store in the page->index becomes stale if the page
gets moved to another free list by move_freepages(). Not sure if that is
a problem for you or not but it is possible that
get_page_migratetype(page) != get_pageblock_migratetype(page)

> ---
>  include/linux/mm.h |6 --
>  mm/page_alloc.c|7 ---
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 86d61d6..8fd32da 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -251,12 +251,14 @@ struct inode;
>  
>  static inline void set_page_migratetype(struct page *page, int migratetype)
>  {
> - set_page_private(page, migratetype);
> + VM_BUG_ON((unsigned int)migratetype >= MIGRATE_TYPES);

This additional bug check is not mentioned in the changelog. Not clear
if it's necessary.

> + page->index = migratetype;
>  }
>  
>  static inline int get_page_migratetype(struct page *page)
>  {
> - return page_private(page);
> + VM_BUG_ON((unsigned int)page->index >= MIGRATE_TYPES);
> + return page->index;
>  }
>  
>  /*
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 103ba66..32985dd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned 
> int order)
>  {
>   unsigned long flags;
>   int wasMlocked = __TestClearPageMlocked(page);
> + int migratetype;
>  
>   if (!free_pages_prepare(page, order))
>   return;
> @@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned 
> int order)
>   if (unlikely(wasMlocked))
>   free_page_mlock(page);
>   __count_vm_events(PGFREE, 1 << order);
> - free_one_page(page_zone(page), page, order,
> - get_pageblock_migratetype(page));
> -
> + migratetype = get_pageblock_migratetype(page);
> + set_page_migratetype(page, migratetype);
> + free_one_page(page_zone(page), page, order, migratetype);
>   local_irq_restore(flags);
>  }
>  

-- 
Mel Gorman
SUSE Labs
--
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 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Minchan Kim

Page allocator doesn't keep migratetype information to page
when the page is freed. This patch remains the information
to freed page's index field which isn't used by free/alloc
preparing so it shouldn't change any behavir except below one.

This patch adds a new call site in __free_pages_ok so it might be
overhead a bit but it's for high order allocation.
So I believe damage isn't hurt.

Signed-off-by: Minchan Kim 
---
 include/linux/mm.h |6 --
 mm/page_alloc.c|7 ---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 86d61d6..8fd32da 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -251,12 +251,14 @@ struct inode;
 
 static inline void set_page_migratetype(struct page *page, int migratetype)
 {
-   set_page_private(page, migratetype);
+   VM_BUG_ON((unsigned int)migratetype >= MIGRATE_TYPES);
+   page->index = migratetype;
 }
 
 static inline int get_page_migratetype(struct page *page)
 {
-   return page_private(page);
+   VM_BUG_ON((unsigned int)page->index >= MIGRATE_TYPES);
+   return page->index;
 }
 
 /*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 103ba66..32985dd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned int 
order)
 {
unsigned long flags;
int wasMlocked = __TestClearPageMlocked(page);
+   int migratetype;
 
if (!free_pages_prepare(page, order))
return;
@@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned int 
order)
if (unlikely(wasMlocked))
free_page_mlock(page);
__count_vm_events(PGFREE, 1 << order);
-   free_one_page(page_zone(page), page, order,
-   get_pageblock_migratetype(page));
-
+   migratetype = get_pageblock_migratetype(page);
+   set_page_migratetype(page, migratetype);
+   free_one_page(page_zone(page), page, order, migratetype);
local_irq_restore(flags);
 }
 
-- 
1.7.9.5

--
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 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Minchan Kim

On Wed, Sep 05, 2012 at 10:25:34AM +0100, Mel Gorman wrote:
 On Wed, Sep 05, 2012 at 04:26:01PM +0900, Minchan Kim wrote:
  Page allocator doesn't keep migratetype information to page
  when the page is freed. This patch remains the information
  to freed page's index field which isn't used by free/alloc
  preparing so it shouldn't change any behavir except below one.
  
 
 This explanation could have been a *LOT* more helpful.
 
 The page allocator caches the pageblock information in page-private while
 it is in the PCP freelists but this is overwritten with the order of the
 page when freed to the buddy allocator. This patch stores the migratetype
 of the page in the page-index field so that it is available at all times.

I will add your comment in my description.

 
  This patch adds a new call site in __free_pages_ok so it might be
  overhead a bit but it's for high order allocation.
  So I believe damage isn't hurt.
  
 
 The additional call to set_page_migratetype() is not heavy. If you were
 adding a new call to get_pageblock_migratetype() or something equally
 expensive I would be more concerned.

I'm lucky to avoid your keen eye. ;)

 
  Signed-off-by: Minchan Kim minc...@kernel.org
 
 The information you store in the page-index becomes stale if the page
 gets moved to another free list by move_freepages(). Not sure if that is
 a problem for you or not but it is possible that
 get_page_migratetype(page) != get_pageblock_migratetype(page)

Thanks for the spot. I have to fix it.

 
  ---
   include/linux/mm.h |6 --
   mm/page_alloc.c|7 ---
   2 files changed, 8 insertions(+), 5 deletions(-)
  
  diff --git a/include/linux/mm.h b/include/linux/mm.h
  index 86d61d6..8fd32da 100644
  --- a/include/linux/mm.h
  +++ b/include/linux/mm.h
  @@ -251,12 +251,14 @@ struct inode;
   
   static inline void set_page_migratetype(struct page *page, int migratetype)
   {
  -   set_page_private(page, migratetype);
  +   VM_BUG_ON((unsigned int)migratetype = MIGRATE_TYPES);
 
 This additional bug check is not mentioned in the changelog. Not clear
 if it's necessary.

I'm not strong so if anyone think it's not necessary, I will drop.

 
  +   page-index = migratetype;
   }
   
   static inline int get_page_migratetype(struct page *page)
   {
  -   return page_private(page);
  +   VM_BUG_ON((unsigned int)page-index = MIGRATE_TYPES);
  +   return page-index;
   }
   
   /*
  diff --git a/mm/page_alloc.c b/mm/page_alloc.c
  index 103ba66..32985dd 100644
  --- a/mm/page_alloc.c
  +++ b/mm/page_alloc.c
  @@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned 
  int order)
   {
  unsigned long flags;
  int wasMlocked = __TestClearPageMlocked(page);
  +   int migratetype;
   
  if (!free_pages_prepare(page, order))
  return;
  @@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned 
  int order)
  if (unlikely(wasMlocked))
  free_page_mlock(page);
  __count_vm_events(PGFREE, 1  order);
  -   free_one_page(page_zone(page), page, order,
  -   get_pageblock_migratetype(page));
  -
  +   migratetype = get_pageblock_migratetype(page);
  +   set_page_migratetype(page, migratetype);
  +   free_one_page(page_zone(page), page, order, migratetype);
  local_irq_restore(flags);
   }
   
 
 -- 
 Mel Gorman
 SUSE Labs
 
 --
 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

-- 
Kind regards,
Minchan Kim
--
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 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Minchan Kim

Page allocator doesn't keep migratetype information to page
when the page is freed. This patch remains the information
to freed page's index field which isn't used by free/alloc
preparing so it shouldn't change any behavir except below one.

This patch adds a new call site in __free_pages_ok so it might be
overhead a bit but it's for high order allocation.
So I believe damage isn't hurt.

Signed-off-by: Minchan Kim minc...@kernel.org
---
 include/linux/mm.h |6 --
 mm/page_alloc.c|7 ---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 86d61d6..8fd32da 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -251,12 +251,14 @@ struct inode;
 
 static inline void set_page_migratetype(struct page *page, int migratetype)
 {
-   set_page_private(page, migratetype);
+   VM_BUG_ON((unsigned int)migratetype = MIGRATE_TYPES);
+   page-index = migratetype;
 }
 
 static inline int get_page_migratetype(struct page *page)
 {
-   return page_private(page);
+   VM_BUG_ON((unsigned int)page-index = MIGRATE_TYPES);
+   return page-index;
 }
 
 /*
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 103ba66..32985dd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned int 
order)
 {
unsigned long flags;
int wasMlocked = __TestClearPageMlocked(page);
+   int migratetype;
 
if (!free_pages_prepare(page, order))
return;
@@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned int 
order)
if (unlikely(wasMlocked))
free_page_mlock(page);
__count_vm_events(PGFREE, 1  order);
-   free_one_page(page_zone(page), page, order,
-   get_pageblock_migratetype(page));
-
+   migratetype = get_pageblock_migratetype(page);
+   set_page_migratetype(page, migratetype);
+   free_one_page(page_zone(page), page, order, migratetype);
local_irq_restore(flags);
 }
 
-- 
1.7.9.5

--
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 2/3] mm: remain migratetype in freed page

2012-09-05 Thread Mel Gorman

On Wed, Sep 05, 2012 at 04:26:01PM +0900, Minchan Kim wrote:
 Page allocator doesn't keep migratetype information to page
 when the page is freed. This patch remains the information
 to freed page's index field which isn't used by free/alloc
 preparing so it shouldn't change any behavir except below one.
 

This explanation could have been a *LOT* more helpful.

The page allocator caches the pageblock information in page-private while
it is in the PCP freelists but this is overwritten with the order of the
page when freed to the buddy allocator. This patch stores the migratetype
of the page in the page-index field so that it is available at all times.

 This patch adds a new call site in __free_pages_ok so it might be
 overhead a bit but it's for high order allocation.
 So I believe damage isn't hurt.
 

The additional call to set_page_migratetype() is not heavy. If you were
adding a new call to get_pageblock_migratetype() or something equally
expensive I would be more concerned.

 Signed-off-by: Minchan Kim minc...@kernel.org

The information you store in the page-index becomes stale if the page
gets moved to another free list by move_freepages(). Not sure if that is
a problem for you or not but it is possible that
get_page_migratetype(page) != get_pageblock_migratetype(page)

 ---
  include/linux/mm.h |6 --
  mm/page_alloc.c|7 ---
  2 files changed, 8 insertions(+), 5 deletions(-)
 
 diff --git a/include/linux/mm.h b/include/linux/mm.h
 index 86d61d6..8fd32da 100644
 --- a/include/linux/mm.h
 +++ b/include/linux/mm.h
 @@ -251,12 +251,14 @@ struct inode;
  
  static inline void set_page_migratetype(struct page *page, int migratetype)
  {
 - set_page_private(page, migratetype);
 + VM_BUG_ON((unsigned int)migratetype = MIGRATE_TYPES);

This additional bug check is not mentioned in the changelog. Not clear
if it's necessary.

 + page-index = migratetype;
  }
  
  static inline int get_page_migratetype(struct page *page)
  {
 - return page_private(page);
 + VM_BUG_ON((unsigned int)page-index = MIGRATE_TYPES);
 + return page-index;
  }
  
  /*
 diff --git a/mm/page_alloc.c b/mm/page_alloc.c
 index 103ba66..32985dd 100644
 --- a/mm/page_alloc.c
 +++ b/mm/page_alloc.c
 @@ -723,6 +723,7 @@ static void __free_pages_ok(struct page *page, unsigned 
 int order)
  {
   unsigned long flags;
   int wasMlocked = __TestClearPageMlocked(page);
 + int migratetype;
  
   if (!free_pages_prepare(page, order))
   return;
 @@ -731,9 +732,9 @@ static void __free_pages_ok(struct page *page, unsigned 
 int order)
   if (unlikely(wasMlocked))
   free_page_mlock(page);
   __count_vm_events(PGFREE, 1  order);
 - free_one_page(page_zone(page), page, order,
 - get_pageblock_migratetype(page));
 -
 + migratetype = get_pageblock_migratetype(page);
 + set_page_migratetype(page, migratetype);
 + free_one_page(page_zone(page), page, order, migratetype);
   local_irq_restore(flags);
  }
  

-- 
Mel Gorman
SUSE Labs
--
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 2/3] mm: remain migratetype in freed page

Re: [PATCH 2/3] mm: remain migratetype in freed page

[PATCH 2/3] mm: remain migratetype in freed page

Re: [PATCH 2/3] mm: remain migratetype in freed page

[PATCH 2/3] mm: remain migratetype in freed page

Re: [PATCH 2/3] mm: remain migratetype in freed page

6 matches

Site Navigation

Mail list logo

Footer information