All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-02-14 18:34 ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-02-14 18:34 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

Pages allocated from MIGRATE_RESERVE migratetype pageblocks
are not freed back to MIGRATE_RESERVE migratetype free
lists in free_pcppages_bulk()->__free_one_page() if we got
to free_pcppages_bulk() through drain_[zone_]pages().
The freeing through free_hot_cold_page() is okay because
freepage migratetype is set to pageblock migratetype before
calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
migratetype end up on the free lists of other migratetype
whole Reserved pageblock may be later changed to the other
migratetype in __rmqueue_fallback() and it will be never
changed back to be a Reserved pageblock.  Fix the issue by
preserving freepage migratetype as a pageblock migratetype
(instead of overriding it to the requested migratetype)
for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().

The problem was introduced in v2.6.31 by commit ed0ae21
("page allocator: do not call get_pageblock_migratetype()
more than necessary").

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
---
v2:
- updated patch description, there is no __zone_pcp_update()
  in newer kernels

 include/linux/mmzone.h |    5 +++++
 mm/page_alloc.c        |   10 +++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

Index: b/include/linux/mmzone.h
===================================================================
--- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
+++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
@@ -63,6 +63,11 @@ enum {
 	MIGRATE_TYPES
 };
 
+static inline bool is_migrate_reserve(int migratetype)
+{
+	return unlikely(migratetype == MIGRATE_RESERVE);
+}
+
 #ifdef CONFIG_CMA
 #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
 #else
Index: b/mm/page_alloc.c
===================================================================
--- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
+++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
@@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
 			unsigned long count, struct list_head *list,
 			int migratetype, int cold)
 {
-	int mt = migratetype, i;
+	int mt, i;
 
 	spin_lock(&zone->lock);
 	for (i = 0; i < count; ++i) {
@@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
 			list_add(&page->lru, list);
 		else
 			list_add_tail(&page->lru, list);
+		mt = get_pageblock_migratetype(page);
 		if (IS_ENABLED(CONFIG_CMA)) {
-			mt = get_pageblock_migratetype(page);
-			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
+			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
+			    !is_migrate_reserve(mt))
+				mt = migratetype;
+		} else {
+			if (!is_migrate_reserve(mt))
 				mt = migratetype;
 		}
 		set_freepage_migratetype(page, mt);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-02-14 18:34 ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-02-14 18:34 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

Pages allocated from MIGRATE_RESERVE migratetype pageblocks
are not freed back to MIGRATE_RESERVE migratetype free
lists in free_pcppages_bulk()->__free_one_page() if we got
to free_pcppages_bulk() through drain_[zone_]pages().
The freeing through free_hot_cold_page() is okay because
freepage migratetype is set to pageblock migratetype before
calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
migratetype end up on the free lists of other migratetype
whole Reserved pageblock may be later changed to the other
migratetype in __rmqueue_fallback() and it will be never
changed back to be a Reserved pageblock.  Fix the issue by
preserving freepage migratetype as a pageblock migratetype
(instead of overriding it to the requested migratetype)
for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().

The problem was introduced in v2.6.31 by commit ed0ae21
("page allocator: do not call get_pageblock_migratetype()
more than necessary").

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Hugh Dickins <hughd@google.com>
---
v2:
- updated patch description, there is no __zone_pcp_update()
  in newer kernels

 include/linux/mmzone.h |    5 +++++
 mm/page_alloc.c        |   10 +++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

Index: b/include/linux/mmzone.h
===================================================================
--- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
+++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
@@ -63,6 +63,11 @@ enum {
 	MIGRATE_TYPES
 };
 
+static inline bool is_migrate_reserve(int migratetype)
+{
+	return unlikely(migratetype == MIGRATE_RESERVE);
+}
+
 #ifdef CONFIG_CMA
 #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
 #else
Index: b/mm/page_alloc.c
===================================================================
--- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
+++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
@@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
 			unsigned long count, struct list_head *list,
 			int migratetype, int cold)
 {
-	int mt = migratetype, i;
+	int mt, i;
 
 	spin_lock(&zone->lock);
 	for (i = 0; i < count; ++i) {
@@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
 			list_add(&page->lru, list);
 		else
 			list_add_tail(&page->lru, list);
+		mt = get_pageblock_migratetype(page);
 		if (IS_ENABLED(CONFIG_CMA)) {
-			mt = get_pageblock_migratetype(page);
-			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
+			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
+			    !is_migrate_reserve(mt))
+				mt = migratetype;
+		} else {
+			if (!is_migrate_reserve(mt))
 				mt = migratetype;
 		}
 		set_freepage_migratetype(page, mt);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
  2014-02-14 18:34 ` Bartlomiej Zolnierkiewicz
@ 2014-02-24  8:59   ` Mel Gorman
  -1 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-02-24  8:59 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> are not freed back to MIGRATE_RESERVE migratetype free
> lists in free_pcppages_bulk()->__free_one_page() if we got
> to free_pcppages_bulk() through drain_[zone_]pages().
> The freeing through free_hot_cold_page() is okay because
> freepage migratetype is set to pageblock migratetype before
> calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> migratetype end up on the free lists of other migratetype
> whole Reserved pageblock may be later changed to the other
> migratetype in __rmqueue_fallback() and it will be never
> changed back to be a Reserved pageblock.  Fix the issue by
> preserving freepage migratetype as a pageblock migratetype
> (instead of overriding it to the requested migratetype)
> for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> 
> The problem was introduced in v2.6.31 by commit ed0ae21
> ("page allocator: do not call get_pageblock_migratetype()
> more than necessary").
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>
> ---
> v2:
> - updated patch description, there is no __zone_pcp_update()
>   in newer kernels
> 
>  include/linux/mmzone.h |    5 +++++
>  mm/page_alloc.c        |   10 +++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> Index: b/include/linux/mmzone.h
> ===================================================================
> --- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
> +++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
> @@ -63,6 +63,11 @@ enum {
>  	MIGRATE_TYPES
>  };
>  
> +static inline bool is_migrate_reserve(int migratetype)
> +{
> +	return unlikely(migratetype == MIGRATE_RESERVE);
> +}
> +
>  #ifdef CONFIG_CMA
>  #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
>  #else
> Index: b/mm/page_alloc.c
> ===================================================================
> --- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
> +++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
> @@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
>  			unsigned long count, struct list_head *list,
>  			int migratetype, int cold)
>  {
> -	int mt = migratetype, i;
> +	int mt, i;
>  
>  	spin_lock(&zone->lock);
>  	for (i = 0; i < count; ++i) {
> @@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
>  			list_add(&page->lru, list);
>  		else
>  			list_add_tail(&page->lru, list);
> +		mt = get_pageblock_migratetype(page);
>  		if (IS_ENABLED(CONFIG_CMA)) {
> -			mt = get_pageblock_migratetype(page);
> -			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
> +			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
> +			    !is_migrate_reserve(mt))
> +				mt = migratetype;
> +		} else {
> +			if (!is_migrate_reserve(mt))
>  				mt = migratetype;

Minimally, this could be simplified because now it's an unconditional
call to get_pageblock_migratetype.

However, it looks like this could be improved without doing that.
__rmqueue_fallback will be called if a page of the requested migratetype
was not found. Furthermore, if a pageblock has been stolen then the
pages are shuffled between free lists so you should be able to modify
this patch to

1. have __rmqueue call set_freepage_migratetype(migratetype) if
   __rmqueue_smallest found a page
2. have __rmqueue_fallback call set_freepage_migratetype(new_type)
   when it has selected which freelist to select from.

Can you check it out as an alternative to this patch please as it would
have much less overhead than unconditionally calling
get_pageblock_migratetype()?

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-02-24  8:59   ` Mel Gorman
  0 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-02-24  8:59 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> are not freed back to MIGRATE_RESERVE migratetype free
> lists in free_pcppages_bulk()->__free_one_page() if we got
> to free_pcppages_bulk() through drain_[zone_]pages().
> The freeing through free_hot_cold_page() is okay because
> freepage migratetype is set to pageblock migratetype before
> calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> migratetype end up on the free lists of other migratetype
> whole Reserved pageblock may be later changed to the other
> migratetype in __rmqueue_fallback() and it will be never
> changed back to be a Reserved pageblock.  Fix the issue by
> preserving freepage migratetype as a pageblock migratetype
> (instead of overriding it to the requested migratetype)
> for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> 
> The problem was introduced in v2.6.31 by commit ed0ae21
> ("page allocator: do not call get_pageblock_migratetype()
> more than necessary").
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>
> ---
> v2:
> - updated patch description, there is no __zone_pcp_update()
>   in newer kernels
> 
>  include/linux/mmzone.h |    5 +++++
>  mm/page_alloc.c        |   10 +++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> Index: b/include/linux/mmzone.h
> ===================================================================
> --- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
> +++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
> @@ -63,6 +63,11 @@ enum {
>  	MIGRATE_TYPES
>  };
>  
> +static inline bool is_migrate_reserve(int migratetype)
> +{
> +	return unlikely(migratetype == MIGRATE_RESERVE);
> +}
> +
>  #ifdef CONFIG_CMA
>  #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
>  #else
> Index: b/mm/page_alloc.c
> ===================================================================
> --- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
> +++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
> @@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
>  			unsigned long count, struct list_head *list,
>  			int migratetype, int cold)
>  {
> -	int mt = migratetype, i;
> +	int mt, i;
>  
>  	spin_lock(&zone->lock);
>  	for (i = 0; i < count; ++i) {
> @@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
>  			list_add(&page->lru, list);
>  		else
>  			list_add_tail(&page->lru, list);
> +		mt = get_pageblock_migratetype(page);
>  		if (IS_ENABLED(CONFIG_CMA)) {
> -			mt = get_pageblock_migratetype(page);
> -			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
> +			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
> +			    !is_migrate_reserve(mt))
> +				mt = migratetype;
> +		} else {
> +			if (!is_migrate_reserve(mt))
>  				mt = migratetype;

Minimally, this could be simplified because now it's an unconditional
call to get_pageblock_migratetype.

However, it looks like this could be improved without doing that.
__rmqueue_fallback will be called if a page of the requested migratetype
was not found. Furthermore, if a pageblock has been stolen then the
pages are shuffled between free lists so you should be able to modify
this patch to

1. have __rmqueue call set_freepage_migratetype(migratetype) if
   __rmqueue_smallest found a page
2. have __rmqueue_fallback call set_freepage_migratetype(new_type)
   when it has selected which freelist to select from.

Can you check it out as an alternative to this patch please as it would
have much less overhead than unconditionally calling
get_pageblock_migratetype()?

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
  2014-02-24  8:59   ` Mel Gorman
@ 2014-03-06 18:12     ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-03-06 18:12 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel


Hi,

On Monday, February 24, 2014 08:59:39 AM Mel Gorman wrote:
> On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > are not freed back to MIGRATE_RESERVE migratetype free
> > lists in free_pcppages_bulk()->__free_one_page() if we got
> > to free_pcppages_bulk() through drain_[zone_]pages().
> > The freeing through free_hot_cold_page() is okay because
> > freepage migratetype is set to pageblock migratetype before
> > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > migratetype end up on the free lists of other migratetype
> > whole Reserved pageblock may be later changed to the other
> > migratetype in __rmqueue_fallback() and it will be never
> > changed back to be a Reserved pageblock.  Fix the issue by
> > preserving freepage migratetype as a pageblock migratetype
> > (instead of overriding it to the requested migratetype)
> > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > 
> > The problem was introduced in v2.6.31 by commit ed0ae21
> > ("page allocator: do not call get_pageblock_migratetype()
> > more than necessary").
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Hugh Dickins <hughd@google.com>
> > ---
> > v2:
> > - updated patch description, there is no __zone_pcp_update()
> >   in newer kernels
> > 
> >  include/linux/mmzone.h |    5 +++++
> >  mm/page_alloc.c        |   10 +++++++---
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > Index: b/include/linux/mmzone.h
> > ===================================================================
> > --- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
> > +++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
> > @@ -63,6 +63,11 @@ enum {
> >  	MIGRATE_TYPES
> >  };
> >  
> > +static inline bool is_migrate_reserve(int migratetype)
> > +{
> > +	return unlikely(migratetype == MIGRATE_RESERVE);
> > +}
> > +
> >  #ifdef CONFIG_CMA
> >  #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
> >  #else
> > Index: b/mm/page_alloc.c
> > ===================================================================
> > --- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
> > +++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
> > @@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
> >  			unsigned long count, struct list_head *list,
> >  			int migratetype, int cold)
> >  {
> > -	int mt = migratetype, i;
> > +	int mt, i;
> >  
> >  	spin_lock(&zone->lock);
> >  	for (i = 0; i < count; ++i) {
> > @@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
> >  			list_add(&page->lru, list);
> >  		else
> >  			list_add_tail(&page->lru, list);
> > +		mt = get_pageblock_migratetype(page);
> >  		if (IS_ENABLED(CONFIG_CMA)) {
> > -			mt = get_pageblock_migratetype(page);
> > -			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
> > +			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
> > +			    !is_migrate_reserve(mt))
> > +				mt = migratetype;
> > +		} else {
> > +			if (!is_migrate_reserve(mt))
> >  				mt = migratetype;
> 
> Minimally, this could be simplified because now it's an unconditional
> call to get_pageblock_migratetype.
> 
> However, it looks like this could be improved without doing that.
> __rmqueue_fallback will be called if a page of the requested migratetype
> was not found. Furthermore, if a pageblock has been stolen then the
> pages are shuffled between free lists so you should be able to modify
> this patch to
> 
> 1. have __rmqueue call set_freepage_migratetype(migratetype) if
>    __rmqueue_smallest found a page
> 2. have __rmqueue_fallback call set_freepage_migratetype(new_type)
>    when it has selected which freelist to select from.
> 
> Can you check it out as an alternative to this patch please as it would
> have much less overhead than unconditionally calling
> get_pageblock_migratetype()?

I updated the patch (please see the other mail) but besides fixing
MIGRATE_RESERVE issue I left the current code behaviour unchanged
for now - freepage migratetype is not set to new_type in
__rmqueue_fallback() as it would affect pages of other migratetypes
(i.e. MIGRATE_MOVABLE or MIGRATE_UNMVOVABLE ones).  I think that
setting freepage migratetype to new_type instead of the requested
migratetype in __rmqueue_fallback() would go beyond the scope of
current patch and I don't know whether it is desirable (I can do
an incremental patch implementing it if needed).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-03-06 18:12     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-03-06 18:12 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel


Hi,

On Monday, February 24, 2014 08:59:39 AM Mel Gorman wrote:
> On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > are not freed back to MIGRATE_RESERVE migratetype free
> > lists in free_pcppages_bulk()->__free_one_page() if we got
> > to free_pcppages_bulk() through drain_[zone_]pages().
> > The freeing through free_hot_cold_page() is okay because
> > freepage migratetype is set to pageblock migratetype before
> > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > migratetype end up on the free lists of other migratetype
> > whole Reserved pageblock may be later changed to the other
> > migratetype in __rmqueue_fallback() and it will be never
> > changed back to be a Reserved pageblock.  Fix the issue by
> > preserving freepage migratetype as a pageblock migratetype
> > (instead of overriding it to the requested migratetype)
> > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > 
> > The problem was introduced in v2.6.31 by commit ed0ae21
> > ("page allocator: do not call get_pageblock_migratetype()
> > more than necessary").
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Hugh Dickins <hughd@google.com>
> > ---
> > v2:
> > - updated patch description, there is no __zone_pcp_update()
> >   in newer kernels
> > 
> >  include/linux/mmzone.h |    5 +++++
> >  mm/page_alloc.c        |   10 +++++++---
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > Index: b/include/linux/mmzone.h
> > ===================================================================
> > --- a/include/linux/mmzone.h	2014-02-14 18:59:08.177837747 +0100
> > +++ b/include/linux/mmzone.h	2014-02-14 18:59:09.077837731 +0100
> > @@ -63,6 +63,11 @@ enum {
> >  	MIGRATE_TYPES
> >  };
> >  
> > +static inline bool is_migrate_reserve(int migratetype)
> > +{
> > +	return unlikely(migratetype == MIGRATE_RESERVE);
> > +}
> > +
> >  #ifdef CONFIG_CMA
> >  #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
> >  #else
> > Index: b/mm/page_alloc.c
> > ===================================================================
> > --- a/mm/page_alloc.c	2014-02-14 18:59:08.185837746 +0100
> > +++ b/mm/page_alloc.c	2014-02-14 18:59:09.077837731 +0100
> > @@ -1174,7 +1174,7 @@ static int rmqueue_bulk(struct zone *zon
> >  			unsigned long count, struct list_head *list,
> >  			int migratetype, int cold)
> >  {
> > -	int mt = migratetype, i;
> > +	int mt, i;
> >  
> >  	spin_lock(&zone->lock);
> >  	for (i = 0; i < count; ++i) {
> > @@ -1195,9 +1195,13 @@ static int rmqueue_bulk(struct zone *zon
> >  			list_add(&page->lru, list);
> >  		else
> >  			list_add_tail(&page->lru, list);
> > +		mt = get_pageblock_migratetype(page);
> >  		if (IS_ENABLED(CONFIG_CMA)) {
> > -			mt = get_pageblock_migratetype(page);
> > -			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt))
> > +			if (!is_migrate_cma(mt) && !is_migrate_isolate(mt) &&
> > +			    !is_migrate_reserve(mt))
> > +				mt = migratetype;
> > +		} else {
> > +			if (!is_migrate_reserve(mt))
> >  				mt = migratetype;
> 
> Minimally, this could be simplified because now it's an unconditional
> call to get_pageblock_migratetype.
> 
> However, it looks like this could be improved without doing that.
> __rmqueue_fallback will be called if a page of the requested migratetype
> was not found. Furthermore, if a pageblock has been stolen then the
> pages are shuffled between free lists so you should be able to modify
> this patch to
> 
> 1. have __rmqueue call set_freepage_migratetype(migratetype) if
>    __rmqueue_smallest found a page
> 2. have __rmqueue_fallback call set_freepage_migratetype(new_type)
>    when it has selected which freelist to select from.
> 
> Can you check it out as an alternative to this patch please as it would
> have much less overhead than unconditionally calling
> get_pageblock_migratetype()?

I updated the patch (please see the other mail) but besides fixing
MIGRATE_RESERVE issue I left the current code behaviour unchanged
for now - freepage migratetype is not set to new_type in
__rmqueue_fallback() as it would affect pages of other migratetypes
(i.e. MIGRATE_MOVABLE or MIGRATE_UNMVOVABLE ones).  I think that
setting freepage migratetype to new_type instead of the requested
migratetype in __rmqueue_fallback() would go beyond the scope of
current patch and I don't know whether it is desirable (I can do
an incremental patch implementing it if needed).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
  2014-02-14 18:34 ` Bartlomiej Zolnierkiewicz
@ 2014-03-13 14:25   ` Mel Gorman
  -1 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-03-13 14:25 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> are not freed back to MIGRATE_RESERVE migratetype free
> lists in free_pcppages_bulk()->__free_one_page() if we got
> to free_pcppages_bulk() through drain_[zone_]pages().
> The freeing through free_hot_cold_page() is okay because
> freepage migratetype is set to pageblock migratetype before
> calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> migratetype end up on the free lists of other migratetype
> whole Reserved pageblock may be later changed to the other
> migratetype in __rmqueue_fallback() and it will be never
> changed back to be a Reserved pageblock.  Fix the issue by
> preserving freepage migratetype as a pageblock migratetype
> (instead of overriding it to the requested migratetype)
> for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> 
> The problem was introduced in v2.6.31 by commit ed0ae21
> ("page allocator: do not call get_pageblock_migratetype()
> more than necessary").
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>

It's a pity about the unconditional pageblock lookup in that path but I
didn't see a better way around it so

Acked-by: Mel Gorman <mgorman@suse.de>

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-03-13 14:25   ` Mel Gorman
  0 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-03-13 14:25 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> are not freed back to MIGRATE_RESERVE migratetype free
> lists in free_pcppages_bulk()->__free_one_page() if we got
> to free_pcppages_bulk() through drain_[zone_]pages().
> The freeing through free_hot_cold_page() is okay because
> freepage migratetype is set to pageblock migratetype before
> calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> migratetype end up on the free lists of other migratetype
> whole Reserved pageblock may be later changed to the other
> migratetype in __rmqueue_fallback() and it will be never
> changed back to be a Reserved pageblock.  Fix the issue by
> preserving freepage migratetype as a pageblock migratetype
> (instead of overriding it to the requested migratetype)
> for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> 
> The problem was introduced in v2.6.31 by commit ed0ae21
> ("page allocator: do not call get_pageblock_migratetype()
> more than necessary").
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Hugh Dickins <hughd@google.com>

It's a pity about the unconditional pageblock lookup in that path but I
didn't see a better way around it so

Acked-by: Mel Gorman <mgorman@suse.de>

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
  2014-03-13 14:25   ` Mel Gorman
@ 2014-03-13 14:37     ` Bartlomiej Zolnierkiewicz
  -1 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-03-13 14:37 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Thursday, March 13, 2014 02:25:40 PM Mel Gorman wrote:
> On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > are not freed back to MIGRATE_RESERVE migratetype free
> > lists in free_pcppages_bulk()->__free_one_page() if we got
> > to free_pcppages_bulk() through drain_[zone_]pages().
> > The freeing through free_hot_cold_page() is okay because
> > freepage migratetype is set to pageblock migratetype before
> > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > migratetype end up on the free lists of other migratetype
> > whole Reserved pageblock may be later changed to the other
> > migratetype in __rmqueue_fallback() and it will be never
> > changed back to be a Reserved pageblock.  Fix the issue by
> > preserving freepage migratetype as a pageblock migratetype
> > (instead of overriding it to the requested migratetype)
> > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > 
> > The problem was introduced in v2.6.31 by commit ed0ae21
> > ("page allocator: do not call get_pageblock_migratetype()
> > more than necessary").
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Hugh Dickins <hughd@google.com>
> 
> It's a pity about the unconditional pageblock lookup in that path but I
> didn't see a better way around it so
> 
> Acked-by: Mel Gorman <mgorman@suse.de>

Thanks but does that mean that v3 should be abandoned:

	https://lkml.org/lkml/2014/3/6/365

?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-03-13 14:37     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 12+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2014-03-13 14:37 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Thursday, March 13, 2014 02:25:40 PM Mel Gorman wrote:
> On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > are not freed back to MIGRATE_RESERVE migratetype free
> > lists in free_pcppages_bulk()->__free_one_page() if we got
> > to free_pcppages_bulk() through drain_[zone_]pages().
> > The freeing through free_hot_cold_page() is okay because
> > freepage migratetype is set to pageblock migratetype before
> > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > migratetype end up on the free lists of other migratetype
> > whole Reserved pageblock may be later changed to the other
> > migratetype in __rmqueue_fallback() and it will be never
> > changed back to be a Reserved pageblock.  Fix the issue by
> > preserving freepage migratetype as a pageblock migratetype
> > (instead of overriding it to the requested migratetype)
> > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > 
> > The problem was introduced in v2.6.31 by commit ed0ae21
> > ("page allocator: do not call get_pageblock_migratetype()
> > more than necessary").
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Hugh Dickins <hughd@google.com>
> 
> It's a pity about the unconditional pageblock lookup in that path but I
> didn't see a better way around it so
> 
> Acked-by: Mel Gorman <mgorman@suse.de>

Thanks but does that mean that v3 should be abandoned:

	https://lkml.org/lkml/2014/3/6/365

?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
  2014-03-13 14:37     ` Bartlomiej Zolnierkiewicz
@ 2014-03-13 15:05       ` Mel Gorman
  -1 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-03-13 15:05 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Thu, Mar 13, 2014 at 03:37:46PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday, March 13, 2014 02:25:40 PM Mel Gorman wrote:
> > On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > > are not freed back to MIGRATE_RESERVE migratetype free
> > > lists in free_pcppages_bulk()->__free_one_page() if we got
> > > to free_pcppages_bulk() through drain_[zone_]pages().
> > > The freeing through free_hot_cold_page() is okay because
> > > freepage migratetype is set to pageblock migratetype before
> > > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > > migratetype end up on the free lists of other migratetype
> > > whole Reserved pageblock may be later changed to the other
> > > migratetype in __rmqueue_fallback() and it will be never
> > > changed back to be a Reserved pageblock.  Fix the issue by
> > > preserving freepage migratetype as a pageblock migratetype
> > > (instead of overriding it to the requested migratetype)
> > > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > > 
> > > The problem was introduced in v2.6.31 by commit ed0ae21
> > > ("page allocator: do not call get_pageblock_migratetype()
> > > more than necessary").
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Cc: Mel Gorman <mgorman@suse.de>
> > > Cc: Hugh Dickins <hughd@google.com>
> > 
> > It's a pity about the unconditional pageblock lookup in that path but I
> > didn't see a better way around it so
> > 
> > Acked-by: Mel Gorman <mgorman@suse.de>
> 
> Thanks but does that mean that v3 should be abandoned:
> 
> 	https://lkml.org/lkml/2014/3/6/365
> 

Bah, no. v3 looks better. I was going through the vast pile of mail marked
unread and thought it must be the latest version if it was still "new"
and didn't search linux-mm. I was obviously thinking more clearly when I
saw v2 the first time.

Sorry for the confusion.

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages
@ 2014-03-13 15:05       ` Mel Gorman
  0 siblings, 0 replies; 12+ messages in thread
From: Mel Gorman @ 2014-03-13 15:05 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Hugh Dickins, Marek Szyprowski, Yong-Taek Lee, linux-mm, linux-kernel

On Thu, Mar 13, 2014 at 03:37:46PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday, March 13, 2014 02:25:40 PM Mel Gorman wrote:
> > On Fri, Feb 14, 2014 at 07:34:17PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > Pages allocated from MIGRATE_RESERVE migratetype pageblocks
> > > are not freed back to MIGRATE_RESERVE migratetype free
> > > lists in free_pcppages_bulk()->__free_one_page() if we got
> > > to free_pcppages_bulk() through drain_[zone_]pages().
> > > The freeing through free_hot_cold_page() is okay because
> > > freepage migratetype is set to pageblock migratetype before
> > > calling free_pcppages_bulk().  If pages of MIGRATE_RESERVE
> > > migratetype end up on the free lists of other migratetype
> > > whole Reserved pageblock may be later changed to the other
> > > migratetype in __rmqueue_fallback() and it will be never
> > > changed back to be a Reserved pageblock.  Fix the issue by
> > > preserving freepage migratetype as a pageblock migratetype
> > > (instead of overriding it to the requested migratetype)
> > > for MIGRATE_RESERVE migratetype pages in rmqueue_bulk().
> > > 
> > > The problem was introduced in v2.6.31 by commit ed0ae21
> > > ("page allocator: do not call get_pageblock_migratetype()
> > > more than necessary").
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > Reported-by: Yong-Taek Lee <ytk.lee@samsung.com>
> > > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Cc: Mel Gorman <mgorman@suse.de>
> > > Cc: Hugh Dickins <hughd@google.com>
> > 
> > It's a pity about the unconditional pageblock lookup in that path but I
> > didn't see a better way around it so
> > 
> > Acked-by: Mel Gorman <mgorman@suse.de>
> 
> Thanks but does that mean that v3 should be abandoned:
> 
> 	https://lkml.org/lkml/2014/3/6/365
> 

Bah, no. v3 looks better. I was going through the vast pile of mail marked
unread and thought it must be the latest version if it was still "new"
and didn't search linux-mm. I was obviously thinking more clearly when I
saw v2 the first time.

Sorry for the confusion.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-03-13 15:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14 18:34 [RFC][PATCH v2] mm/page_alloc: fix freeing of MIGRATE_RESERVE migratetype pages Bartlomiej Zolnierkiewicz
2014-02-14 18:34 ` Bartlomiej Zolnierkiewicz
2014-02-24  8:59 ` Mel Gorman
2014-02-24  8:59   ` Mel Gorman
2014-03-06 18:12   ` Bartlomiej Zolnierkiewicz
2014-03-06 18:12     ` Bartlomiej Zolnierkiewicz
2014-03-13 14:25 ` Mel Gorman
2014-03-13 14:25   ` Mel Gorman
2014-03-13 14:37   ` Bartlomiej Zolnierkiewicz
2014-03-13 14:37     ` Bartlomiej Zolnierkiewicz
2014-03-13 15:05     ` Mel Gorman
2014-03-13 15:05       ` Mel Gorman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.