linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc
@ 2012-09-12  1:43 Minchan Kim
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Minchan Kim @ 2012-09-12  1:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Minchan Kim, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Michal Nazarewicz, Marek Szyprowski,
	Wen Congyang

__alloc_contig_migrate_alloc can be used by memory-hotplug so
refactor out(move + rename as a common name) it into
page_isolation.c.

Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---

This patch is intended for preparing next bug fix patch.

 include/linux/page-isolation.h |    3 ++-
 mm/page_alloc.c                |   14 +-------------
 mm/page_isolation.c            |   11 +++++++++++
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 105077a..1c82261 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -37,6 +37,7 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn);
  */
 int set_migratetype_isolate(struct page *page);
 void unset_migratetype_isolate(struct page *page, unsigned migratetype);
-
+struct page *alloc_migrate_target(struct page *page, unsigned long private,
+				int **resultp);
 
 #endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a4ff74e..6716023 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5648,18 +5648,6 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
 				pageblock_nr_pages));
 }
 
-static struct page *
-__alloc_contig_migrate_alloc(struct page *page, unsigned long private,
-			     int **resultp)
-{
-	gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
-
-	if (PageHighMem(page))
-		gfp_mask |= __GFP_HIGHMEM;
-
-	return alloc_page(gfp_mask);
-}
-
 /* [start, end) must belong to a single zone. */
 static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
 {
@@ -5700,7 +5688,7 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
 		}
 
 		ret = migrate_pages(&cc.migratepages,
-				    __alloc_contig_migrate_alloc,
+				    alloc_migrate_target,
 				    0, false, MIGRATE_SYNC);
 	}
 
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 247d1f1..6936545 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -233,3 +233,14 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)
 	spin_unlock_irqrestore(&zone->lock, flags);
 	return ret ? 0 : -EBUSY;
 }
+
+struct page *alloc_migrate_target(struct page *page, unsigned long private,
+                             int **resultp)
+{
+        gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
+
+        if (PageHighMem(page))
+                gfp_mask |= __GFP_HIGHMEM;
+
+        return alloc_page(gfp_mask);
+}
-- 
1.7.9.5


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

* [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
@ 2012-09-12  1:43 ` Minchan Kim
  2012-09-12 10:34   ` Michal Nazarewicz
                     ` (3 more replies)
  2012-09-12 10:32 ` [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Michal Nazarewicz
                   ` (3 subsequent siblings)
  4 siblings, 4 replies; 11+ messages in thread
From: Minchan Kim @ 2012-09-12  1:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Minchan Kim, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Michal Nazarewicz, Marek Szyprowski,
	Wen Congyang

[1] reporeted that lowmem pages could be replaced by
highmem pages during migration of CMA and fixed.

Quote from [1]'s description
"
    The filesystem layer expects pages in the block device's mapping to not
    be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
    currently replace lowmem pages with highmem pages, leading to crashes in
    filesystem code such as the one below:

      Unable to handle kernel NULL pointer dereference at virtual address 00000400
      pgd = c0c98000
      [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
      Internal error: Oops: 817 [#1] PREEMPT SMP ARM
      CPU: 0    Not tainted  (3.5.0-rc5+ #80)
      PC is at __memzero+0x24/0x80
      ...
      Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
      Backtrace:
      [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
      [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
       r4:c15337f0
      [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
      [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
       r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
      [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
       r6:beccdcf0 r5:00074000 r4:beccdbbc
      [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
"

Memory-hotplug has same problem with CMA so [1]'s fix could be applied
with memory-hotplug, too.

Fix it by reusing.

[1] 6a6dccba2, mm: cma: don't replace lowmem pages with highmem

Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/memory_hotplug.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 4491a6b..fb71e5c 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -752,13 +752,6 @@ static unsigned long scan_lru_pages(unsigned long start, unsigned long end)
 	return 0;
 }
 
-static struct page *
-hotremove_migrate_alloc(struct page *page, unsigned long private, int **x)
-{
-	/* This should be improooooved!! */
-	return alloc_page(GFP_HIGHUSER_MOVABLE);
-}
-
 #define NR_OFFLINE_AT_ONCE_PAGES	(256)
 static int
 do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
@@ -809,8 +802,12 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 			putback_lru_pages(&source);
 			goto out;
 		}
-		/* this function returns # of failed pages */
-		ret = migrate_pages(&source, hotremove_migrate_alloc, 0,
+
+		/*
+		 * alloc_migrate_target should be improooooved!!
+		 * migrate_pages returns # of failed pages.
+		 */
+		ret = migrate_pages(&source, alloc_migrate_target, 0,
 							true, MIGRATE_SYNC);
 		if (ret)
 			putback_lru_pages(&source);
-- 
1.7.9.5


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

* Re: [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc
  2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
@ 2012-09-12 10:32 ` Michal Nazarewicz
  2012-09-13  1:57 ` Yasuaki Ishimatsu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Michal Nazarewicz @ 2012-09-12 10:32 UTC (permalink / raw)
  To: Minchan Kim, Andrew Morton
  Cc: linux-mm, linux-kernel, Minchan Kim, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Marek Szyprowski, Wen Congyang

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

On Wed, Sep 12 2012, Minchan Kim <minchan@kernel.org> wrote:
> __alloc_contig_migrate_alloc can be used by memory-hotplug so
> refactor out(move + rename as a common name) it into
> page_isolation.c.
>
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
@ 2012-09-12 10:34   ` Michal Nazarewicz
  2012-09-12 21:32   ` Andrew Morton
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Michal Nazarewicz @ 2012-09-12 10:34 UTC (permalink / raw)
  To: Minchan Kim, Andrew Morton
  Cc: linux-mm, linux-kernel, Minchan Kim, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Marek Szyprowski, Wen Congyang

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

On Wed, Sep 12 2012, Minchan Kim wrote:
> [1] reporeted that lowmem pages could be replaced by
> highmem pages during migration of CMA and fixed.
>
> Quote from [1]'s description
> "
>     The filesystem layer expects pages in the block device's mapping to not
>     be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
>     currently replace lowmem pages with highmem pages, leading to crashes in
>     filesystem code such as the one below:
>
>       Unable to handle kernel NULL pointer dereference at virtual address 00000400
>       pgd = c0c98000
>       [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
>       Internal error: Oops: 817 [#1] PREEMPT SMP ARM
>       CPU: 0    Not tainted  (3.5.0-rc5+ #80)
>       PC is at __memzero+0x24/0x80
>       ...
>       Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
>       Backtrace:
>       [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
>       [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
>        r4:c15337f0
>       [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
>       [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
>        r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
>       [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
>        r6:beccdcf0 r5:00074000 r4:beccdbbc
>       [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
> "
>
> Memory-hotplug has same problem with CMA so [1]'s fix could be applied
> with memory-hotplug, too.
>
> Fix it by reusing.
>
> [1] 6a6dccba2, mm: cma: don't replace lowmem pages with highmem
>
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>

Looks legit:

Acked-by: Michal Nazarewicz <mina86@mina86.com>

I also like how both of the patches cumulatively have negative delta. :]

> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

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

* Re: [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
  2012-09-12 10:34   ` Michal Nazarewicz
@ 2012-09-12 21:32   ` Andrew Morton
  2012-09-13  0:05     ` Minchan Kim
  2012-09-13  1:58   ` Yasuaki Ishimatsu
  2012-09-13  2:16   ` David Rientjes
  3 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2012-09-12 21:32 UTC (permalink / raw)
  To: Minchan Kim
  Cc: linux-mm, linux-kernel, Kamezawa Hiroyuki, Yasuaki Ishimatsu,
	Michal Nazarewicz, Marek Szyprowski, Wen Congyang

On Wed, 12 Sep 2012 10:43:51 +0900
Minchan Kim <minchan@kernel.org> wrote:

> [1] reporeted that lowmem pages could be replaced by
> highmem pages during migration of CMA and fixed.
> 
> Quote from [1]'s description
> "
>     The filesystem layer expects pages in the block device's mapping to not
>     be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
>     currently replace lowmem pages with highmem pages, leading to crashes in
>     filesystem code such as the one below:
> 
>       Unable to handle kernel NULL pointer dereference at virtual address 00000400
>       pgd = c0c98000
>       [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
>       Internal error: Oops: 817 [#1] PREEMPT SMP ARM
>       CPU: 0    Not tainted  (3.5.0-rc5+ #80)
>       PC is at __memzero+0x24/0x80
>       ...
>       Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
>       Backtrace:
>       [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
>       [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
>        r4:c15337f0
>       [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
>       [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
>        r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
>       [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
>        r6:beccdcf0 r5:00074000 r4:beccdbbc
>       [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
> "
> 
> Memory-hotplug has same problem with CMA so [1]'s fix could be applied
> with memory-hotplug, too.
> 
> Fix it by reusing.

Do we think this issue should be fixed in 3.6?  Earlier?

> @@ -809,8 +802,12 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>  			putback_lru_pages(&source);
>  			goto out;
>  		}
> -		/* this function returns # of failed pages */
> -		ret = migrate_pages(&source, hotremove_migrate_alloc, 0,
> +
> +		/*
> +		 * alloc_migrate_target should be improooooved!!

Not a helpful comment!  If you've identified some improvement then
please do provide all the details.

> +		 * migrate_pages returns # of failed pages.
> +		 */
> +		ret = migrate_pages(&source, alloc_migrate_target, 0,
>  							true, MIGRATE_SYNC);
>  		if (ret)
>  			putback_lru_pages(&source);


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

* Re: [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12 21:32   ` Andrew Morton
@ 2012-09-13  0:05     ` Minchan Kim
  0 siblings, 0 replies; 11+ messages in thread
From: Minchan Kim @ 2012-09-13  0:05 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Kamezawa Hiroyuki, Yasuaki Ishimatsu,
	Michal Nazarewicz, Marek Szyprowski, Wen Congyang

On Wed, Sep 12, 2012 at 02:32:39PM -0700, Andrew Morton wrote:
> On Wed, 12 Sep 2012 10:43:51 +0900
> Minchan Kim <minchan@kernel.org> wrote:
> 
> > [1] reporeted that lowmem pages could be replaced by
> > highmem pages during migration of CMA and fixed.
> > 
> > Quote from [1]'s description
> > "
> >     The filesystem layer expects pages in the block device's mapping to not
> >     be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
> >     currently replace lowmem pages with highmem pages, leading to crashes in
> >     filesystem code such as the one below:
> > 
> >       Unable to handle kernel NULL pointer dereference at virtual address 00000400
> >       pgd = c0c98000
> >       [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
> >       Internal error: Oops: 817 [#1] PREEMPT SMP ARM
> >       CPU: 0    Not tainted  (3.5.0-rc5+ #80)
> >       PC is at __memzero+0x24/0x80
> >       ...
> >       Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
> >       Backtrace:
> >       [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
> >       [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
> >        r4:c15337f0
> >       [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
> >       [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
> >        r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
> >       [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
> >        r6:beccdcf0 r5:00074000 r4:beccdbbc
> >       [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
> > "
> > 
> > Memory-hotplug has same problem with CMA so [1]'s fix could be applied
> > with memory-hotplug, too.
> > 
> > Fix it by reusing.
> 
> Do we think this issue should be fixed in 3.6?  Earlier?

I really wanted to Cced stable but didn't due to a below

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).

We haven't ever seen at the report of memory-hotplug although it pops up in CMA.
But I doubt fujitsu guys already saw it.


> 
> > @@ -809,8 +802,12 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> >  			putback_lru_pages(&source);
> >  			goto out;
> >  		}
> > -		/* this function returns # of failed pages */
> > -		ret = migrate_pages(&source, hotremove_migrate_alloc, 0,
> > +
> > +		/*
> > +		 * alloc_migrate_target should be improooooved!!
> 
> Not a helpful comment!  If you've identified some improvement then
> please do provide all the details.

I have an idea to improve it and hoping send patch soonish.
I will handle it in my patch.

Thanks Andrew.

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc
  2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
  2012-09-12 10:32 ` [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Michal Nazarewicz
@ 2012-09-13  1:57 ` Yasuaki Ishimatsu
  2012-09-13  2:14 ` David Rientjes
  2012-09-14  6:32 ` Wen Congyang
  4 siblings, 0 replies; 11+ messages in thread
From: Yasuaki Ishimatsu @ 2012-09-13  1:57 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kamezawa Hiroyuki,
	Michal Nazarewicz, Marek Szyprowski, Wen Congyang

2012/09/12 10:43, Minchan Kim wrote:
> __alloc_contig_migrate_alloc can be used by memory-hotplug so
> refactor out(move + rename as a common name) it into
> page_isolation.c.
> 
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
> 
> This patch is intended for preparing next bug fix patch.
> 
>   include/linux/page-isolation.h |    3 ++-
>   mm/page_alloc.c                |   14 +-------------
>   mm/page_isolation.c            |   11 +++++++++++
>   3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 105077a..1c82261 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -37,6 +37,7 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn);
>    */
>   int set_migratetype_isolate(struct page *page);
>   void unset_migratetype_isolate(struct page *page, unsigned migratetype);
> -
> +struct page *alloc_migrate_target(struct page *page, unsigned long private,
> +				int **resultp);
>   
>   #endif
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a4ff74e..6716023 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5648,18 +5648,6 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
>   				pageblock_nr_pages));
>   }
>   
> -static struct page *
> -__alloc_contig_migrate_alloc(struct page *page, unsigned long private,
> -			     int **resultp)
> -{
> -	gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
> -
> -	if (PageHighMem(page))
> -		gfp_mask |= __GFP_HIGHMEM;
> -
> -	return alloc_page(gfp_mask);
> -}
> -
>   /* [start, end) must belong to a single zone. */
>   static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
>   {
> @@ -5700,7 +5688,7 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
>   		}
>   
>   		ret = migrate_pages(&cc.migratepages,
> -				    __alloc_contig_migrate_alloc,
> +				    alloc_migrate_target,
>   				    0, false, MIGRATE_SYNC);
>   	}
>   
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 247d1f1..6936545 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -233,3 +233,14 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)
>   	spin_unlock_irqrestore(&zone->lock, flags);
>   	return ret ? 0 : -EBUSY;
>   }
> +
> +struct page *alloc_migrate_target(struct page *page, unsigned long private,
> +                             int **resultp)
> +{
> +        gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
> +
> +        if (PageHighMem(page))
> +                gfp_mask |= __GFP_HIGHMEM;
> +
> +        return alloc_page(gfp_mask);
> +}
> 



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

* Re: [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
  2012-09-12 10:34   ` Michal Nazarewicz
  2012-09-12 21:32   ` Andrew Morton
@ 2012-09-13  1:58   ` Yasuaki Ishimatsu
  2012-09-13  2:16   ` David Rientjes
  3 siblings, 0 replies; 11+ messages in thread
From: Yasuaki Ishimatsu @ 2012-09-13  1:58 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kamezawa Hiroyuki,
	Michal Nazarewicz, Marek Szyprowski, Wen Congyang

2012/09/12 10:43, Minchan Kim wrote:
> [1] reporeted that lowmem pages could be replaced by
> highmem pages during migration of CMA and fixed.
> 
> Quote from [1]'s description
> "
>      The filesystem layer expects pages in the block device's mapping to not
>      be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
>      currently replace lowmem pages with highmem pages, leading to crashes in
>      filesystem code such as the one below:
> 
>        Unable to handle kernel NULL pointer dereference at virtual address 00000400
>        pgd = c0c98000
>        [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
>        Internal error: Oops: 817 [#1] PREEMPT SMP ARM
>        CPU: 0    Not tainted  (3.5.0-rc5+ #80)
>        PC is at __memzero+0x24/0x80
>        ...
>        Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
>        Backtrace:
>        [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
>        [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
>         r4:c15337f0
>        [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
>        [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
>         r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
>        [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
>         r6:beccdcf0 r5:00074000 r4:beccdbbc
>        [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
> "
> 
> Memory-hotplug has same problem with CMA so [1]'s fix could be applied
> with memory-hotplug, too.
> 
> Fix it by reusing.
> 
> [1] 6a6dccba2, mm: cma: don't replace lowmem pages with highmem
> 
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>   mm/memory_hotplug.c |   15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4491a6b..fb71e5c 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -752,13 +752,6 @@ static unsigned long scan_lru_pages(unsigned long start, unsigned long end)
>   	return 0;
>   }
>   
> -static struct page *
> -hotremove_migrate_alloc(struct page *page, unsigned long private, int **x)
> -{
> -	/* This should be improooooved!! */
> -	return alloc_page(GFP_HIGHUSER_MOVABLE);
> -}
> -
>   #define NR_OFFLINE_AT_ONCE_PAGES	(256)
>   static int
>   do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> @@ -809,8 +802,12 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>   			putback_lru_pages(&source);
>   			goto out;
>   		}
> -		/* this function returns # of failed pages */
> -		ret = migrate_pages(&source, hotremove_migrate_alloc, 0,
> +
> +		/*
> +		 * alloc_migrate_target should be improooooved!!
> +		 * migrate_pages returns # of failed pages.
> +		 */
> +		ret = migrate_pages(&source, alloc_migrate_target, 0,
>   							true, MIGRATE_SYNC);
>   		if (ret)
>   			putback_lru_pages(&source);
> 



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

* Re: [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc
  2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
                   ` (2 preceding siblings ...)
  2012-09-13  1:57 ` Yasuaki Ishimatsu
@ 2012-09-13  2:14 ` David Rientjes
  2012-09-14  6:32 ` Wen Congyang
  4 siblings, 0 replies; 11+ messages in thread
From: David Rientjes @ 2012-09-13  2:14 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Michal Nazarewicz, Marek Szyprowski,
	Wen Congyang

On Wed, 12 Sep 2012, Minchan Kim wrote:

> __alloc_contig_migrate_alloc can be used by memory-hotplug so
> refactor out(move + rename as a common name) it into
> page_isolation.c.
> 
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Acked-by: David Rientjes <rientjes@google.com>

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

* Re: [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem
  2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
                     ` (2 preceding siblings ...)
  2012-09-13  1:58   ` Yasuaki Ishimatsu
@ 2012-09-13  2:16   ` David Rientjes
  3 siblings, 0 replies; 11+ messages in thread
From: David Rientjes @ 2012-09-13  2:16 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Michal Nazarewicz, Marek Szyprowski,
	Wen Congyang

On Wed, 12 Sep 2012, Minchan Kim wrote:

> [1] reporeted that lowmem pages could be replaced by
> highmem pages during migration of CMA and fixed.
> 
> Quote from [1]'s description
> "
>     The filesystem layer expects pages in the block device's mapping to not
>     be in highmem (the mapping's gfp mask is set in bdget()), but CMA can
>     currently replace lowmem pages with highmem pages, leading to crashes in
>     filesystem code such as the one below:
> 
>       Unable to handle kernel NULL pointer dereference at virtual address 00000400
>       pgd = c0c98000
>       [00000400] *pgd=00c91831, *pte=00000000, *ppte=00000000
>       Internal error: Oops: 817 [#1] PREEMPT SMP ARM
>       CPU: 0    Not tainted  (3.5.0-rc5+ #80)
>       PC is at __memzero+0x24/0x80
>       ...
>       Process fsstress (pid: 323, stack limit = 0xc0cbc2f0)
>       Backtrace:
>       [<c010e3f0>] (ext4_getblk+0x0/0x180) from [<c010e58c>] (ext4_bread+0x1c/0x98)
>       [<c010e570>] (ext4_bread+0x0/0x98) from [<c0117944>] (ext4_mkdir+0x160/0x3bc)
>        r4:c15337f0
>       [<c01177e4>] (ext4_mkdir+0x0/0x3bc) from [<c00c29e0>] (vfs_mkdir+0x8c/0x98)
>       [<c00c2954>] (vfs_mkdir+0x0/0x98) from [<c00c2a60>] (sys_mkdirat+0x74/0xac)
>        r6:00000000 r5:c152eb40 r4:000001ff r3:c14b43f0
>       [<c00c29ec>] (sys_mkdirat+0x0/0xac) from [<c00c2ab8>] (sys_mkdir+0x20/0x24)
>        r6:beccdcf0 r5:00074000 r4:beccdbbc
>       [<c00c2a98>] (sys_mkdir+0x0/0x24) from [<c000e3c0>] (ret_fast_syscall+0x0/0x30)
> "
> 
> Memory-hotplug has same problem with CMA so [1]'s fix could be applied
> with memory-hotplug, too.
> 
> Fix it by reusing.
> 
> [1] 6a6dccba2, mm: cma: don't replace lowmem pages with highmem
> 
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Acked-by: David Rientjes <rientjes@google.com>

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

* Re: [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc
  2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
                   ` (3 preceding siblings ...)
  2012-09-13  2:14 ` David Rientjes
@ 2012-09-14  6:32 ` Wen Congyang
  4 siblings, 0 replies; 11+ messages in thread
From: Wen Congyang @ 2012-09-14  6:32 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm, linux-kernel, Kamezawa Hiroyuki,
	Yasuaki Ishimatsu, Michal Nazarewicz, Marek Szyprowski

At 09/12/2012 09:43 AM, Minchan Kim Wrote:
> __alloc_contig_migrate_alloc can be used by memory-hotplug so
> refactor out(move + rename as a common name) it into
> page_isolation.c.
> 
> Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
> 
> This patch is intended for preparing next bug fix patch.
> 
>  include/linux/page-isolation.h |    3 ++-
>  mm/page_alloc.c                |   14 +-------------
>  mm/page_isolation.c            |   11 +++++++++++
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 105077a..1c82261 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -37,6 +37,7 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn);
>   */
>  int set_migratetype_isolate(struct page *page);
>  void unset_migratetype_isolate(struct page *page, unsigned migratetype);
> -
> +struct page *alloc_migrate_target(struct page *page, unsigned long private,
> +				int **resultp);
>  
>  #endif
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a4ff74e..6716023 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5648,18 +5648,6 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
>  				pageblock_nr_pages));
>  }
>  
> -static struct page *
> -__alloc_contig_migrate_alloc(struct page *page, unsigned long private,
> -			     int **resultp)
> -{
> -	gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
> -
> -	if (PageHighMem(page))
> -		gfp_mask |= __GFP_HIGHMEM;
> -
> -	return alloc_page(gfp_mask);
> -}
> -
>  /* [start, end) must belong to a single zone. */
>  static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
>  {
> @@ -5700,7 +5688,7 @@ static int __alloc_contig_migrate_range(unsigned long start, unsigned long end)
>  		}
>  
>  		ret = migrate_pages(&cc.migratepages,
> -				    __alloc_contig_migrate_alloc,
> +				    alloc_migrate_target,
>  				    0, false, MIGRATE_SYNC);
>  	}
>  
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 247d1f1..6936545 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -233,3 +233,14 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn)
>  	spin_unlock_irqrestore(&zone->lock, flags);
>  	return ret ? 0 : -EBUSY;
>  }
> +
> +struct page *alloc_migrate_target(struct page *page, unsigned long private,
> +                             int **resultp)
> +{
> +        gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
> +
> +        if (PageHighMem(page))
> +                gfp_mask |= __GFP_HIGHMEM;
> +
> +        return alloc_page(gfp_mask);

alloc_page() will use current task's memory policy. If we offline memory like this:
numactl -m n echo offline >/sys/devices/system/memory/memoryX/state # n is page's nid

It may trigger OOM event.

Thanks
Wen Congyang


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

end of thread, other threads:[~2012-09-14  6:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-12  1:43 [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Minchan Kim
2012-09-12  1:43 ` [PATCH 2/2] memory-hotplug: don't replace lowmem pages with highmem Minchan Kim
2012-09-12 10:34   ` Michal Nazarewicz
2012-09-12 21:32   ` Andrew Morton
2012-09-13  0:05     ` Minchan Kim
2012-09-13  1:58   ` Yasuaki Ishimatsu
2012-09-13  2:16   ` David Rientjes
2012-09-12 10:32 ` [PATCH 1/2] mm: refactor out __alloc_contig_migrate_alloc Michal Nazarewicz
2012-09-13  1:57 ` Yasuaki Ishimatsu
2012-09-13  2:14 ` David Rientjes
2012-09-14  6:32 ` Wen Congyang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).