linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
@ 2013-02-04 10:27 Marek Szyprowski
  2013-02-04 23:06 ` Andrew Morton
  2013-02-04 23:34 ` Minchan Kim
  0 siblings, 2 replies; 16+ messages in thread
From: Marek Szyprowski @ 2013-02-04 10:27 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: m.szyprowski, akpm, minchan, mgorman, kyungmin.park

The total number of low memory pages is determined as
totalram_pages - totalhigh_pages, so without this patch all CMA
pageblocks placed in highmem were accounted to low memory.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 mm/page_alloc.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f5bab0a..6415d93 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct page *page)
 	set_pageblock_migratetype(page, MIGRATE_CMA);
 	__free_pages(page, pageblock_order);
 	totalram_pages += pageblock_nr_pages;
+#ifdef CONFIG_HIGHMEM
+	if (PageHighMem(page))
+		totalhigh_pages += pageblock_nr_pages;
+#endif
 }
 #endif
 
-- 
1.7.9.5


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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 10:27 [PATCH] mm: cma: fix accounting of CMA pages placed in high memory Marek Szyprowski
@ 2013-02-04 23:06 ` Andrew Morton
  2013-02-04 23:29   ` Kyungmin Park
  2013-02-04 23:34 ` Minchan Kim
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2013-02-04 23:06 UTC (permalink / raw)
  To: Marek Szyprowski; +Cc: linux-mm, linux-kernel, minchan, mgorman, kyungmin.park

On Mon, 04 Feb 2013 11:27:05 +0100
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> The total number of low memory pages is determined as
> totalram_pages - totalhigh_pages, so without this patch all CMA
> pageblocks placed in highmem were accounted to low memory.

What are the end-user-visible effects of this bug?

(This information is needed so that others can make patch-scheduling
decisions and should be included in all bugfix changelogs unless it is
obvious).


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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:06 ` Andrew Morton
@ 2013-02-04 23:29   ` Kyungmin Park
  2013-02-04 23:43     ` Minchan Kim
  2013-02-05  8:28     ` Mel Gorman
  0 siblings, 2 replies; 16+ messages in thread
From: Kyungmin Park @ 2013-02-04 23:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Marek Szyprowski, linux-mm, linux-kernel, minchan, mgorman

On Tue, Feb 5, 2013 at 8:06 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Mon, 04 Feb 2013 11:27:05 +0100
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
>> The total number of low memory pages is determined as
>> totalram_pages - totalhigh_pages, so without this patch all CMA
>> pageblocks placed in highmem were accounted to low memory.
>
> What are the end-user-visible effects of this bug?

Even though CMA is located at highmem. LowTotal has more than lowmem
address spaces.

e.g.,
lowmem  : 0xc0000000 - 0xdf000000   ( 496 MB)
LowTotal:         555788 kB

>
> (This information is needed so that others can make patch-scheduling
> decisions and should be included in all bugfix changelogs unless it is
> obvious).

CMA Highmem support is new feature. so don't need to go stable tree.

Thank you,
Kyungmin Park
>
> --
> 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] 16+ messages in thread

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 10:27 [PATCH] mm: cma: fix accounting of CMA pages placed in high memory Marek Szyprowski
  2013-02-04 23:06 ` Andrew Morton
@ 2013-02-04 23:34 ` Minchan Kim
  2013-02-05  7:10   ` Marek Szyprowski
  1 sibling, 1 reply; 16+ messages in thread
From: Minchan Kim @ 2013-02-04 23:34 UTC (permalink / raw)
  To: Marek Szyprowski; +Cc: linux-mm, linux-kernel, akpm, mgorman, kyungmin.park

On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
> The total number of low memory pages is determined as
> totalram_pages - totalhigh_pages, so without this patch all CMA
> pageblocks placed in highmem were accounted to low memory.

So what's the end user effect? With the effect, we have to decide
routing it on stable.

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  mm/page_alloc.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f5bab0a..6415d93 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct page *page)
>  	set_pageblock_migratetype(page, MIGRATE_CMA);
>  	__free_pages(page, pageblock_order);
>  	totalram_pages += pageblock_nr_pages;
> +#ifdef CONFIG_HIGHMEM

We don't need #ifdef/#endif.

> +	if (PageHighMem(page))
> +		totalhigh_pages += pageblock_nr_pages;
> +#endif
>  }
>  #endif
>  
> -- 
> 1.7.9.5
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:29   ` Kyungmin Park
@ 2013-02-04 23:43     ` Minchan Kim
  2013-02-04 23:52       ` Kyungmin Park
  2013-02-05  8:28     ` Mel Gorman
  1 sibling, 1 reply; 16+ messages in thread
From: Minchan Kim @ 2013-02-04 23:43 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Andrew Morton, Marek Szyprowski, linux-mm, linux-kernel, mgorman

Hello,

On Tue, Feb 05, 2013 at 08:29:26AM +0900, Kyungmin Park wrote:
> On Tue, Feb 5, 2013 at 8:06 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Mon, 04 Feb 2013 11:27:05 +0100
> > Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> >> The total number of low memory pages is determined as
> >> totalram_pages - totalhigh_pages, so without this patch all CMA
> >> pageblocks placed in highmem were accounted to low memory.
> >
> > What are the end-user-visible effects of this bug?
> 
> Even though CMA is located at highmem. LowTotal has more than lowmem
> address spaces.
> 
> e.g.,
> lowmem  : 0xc0000000 - 0xdf000000   ( 496 MB)
> LowTotal:         555788 kB
> 
> >
> > (This information is needed so that others can make patch-scheduling
> > decisions and should be included in all bugfix changelogs unless it is
> > obvious).
> 
> CMA Highmem support is new feature. so don't need to go stable tree.

I would like to clarify it because I remembered alloc_migrate_target have considered
CMA pages could be highmem. Is it really new feature? If so, could you point out
enabling patches for the new feature?

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);
}

Thanks.

> 
> Thank you,
> Kyungmin Park
> >
> > --
> > 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>
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:43     ` Minchan Kim
@ 2013-02-04 23:52       ` Kyungmin Park
  2013-02-05  0:40         ` Minchan Kim
  0 siblings, 1 reply; 16+ messages in thread
From: Kyungmin Park @ 2013-02-04 23:52 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Marek Szyprowski, linux-mm, linux-kernel, mgorman

Hi,

On Tue, Feb 5, 2013 at 8:43 AM, Minchan Kim <minchan@kernel.org> wrote:
> Hello,
>
> On Tue, Feb 05, 2013 at 08:29:26AM +0900, Kyungmin Park wrote:
>> On Tue, Feb 5, 2013 at 8:06 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > On Mon, 04 Feb 2013 11:27:05 +0100
>> > Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> >
>> >> The total number of low memory pages is determined as
>> >> totalram_pages - totalhigh_pages, so without this patch all CMA
>> >> pageblocks placed in highmem were accounted to low memory.
>> >
>> > What are the end-user-visible effects of this bug?
>>
>> Even though CMA is located at highmem. LowTotal has more than lowmem
>> address spaces.
>>
>> e.g.,
>> lowmem  : 0xc0000000 - 0xdf000000   ( 496 MB)
>> LowTotal:         555788 kB
>>
>> >
>> > (This information is needed so that others can make patch-scheduling
>> > decisions and should be included in all bugfix changelogs unless it is
>> > obvious).
>>
>> CMA Highmem support is new feature. so don't need to go stable tree.
>
> I would like to clarify it because I remembered alloc_migrate_target have considered
> CMA pages could be highmem. Is it really new feature? If so, could you point out
> enabling patches for the new feature?
>
Here's related patch.
http://www.spinics.net/lists/arm-kernel/msg222369.html

Previous time, it's not fully tested and now we checked it with
highmem support patches.

Thank you,
Kyungmin Park
> 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);
> }
>


> Thanks.
>
>>
>> Thank you,
>> Kyungmin Park
>> >
>> > --
>> > 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>
>>
>> --
>> 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>
>
> --
> Kind regards,
> Minchan Kim
>
> --
> 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] 16+ messages in thread

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:52       ` Kyungmin Park
@ 2013-02-05  0:40         ` Minchan Kim
  2013-02-05  8:38           ` Marek Szyprowski
  0 siblings, 1 reply; 16+ messages in thread
From: Minchan Kim @ 2013-02-05  0:40 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Andrew Morton, Marek Szyprowski, linux-mm, linux-kernel, mgorman

On Tue, Feb 05, 2013 at 08:52:02AM +0900, Kyungmin Park wrote:
> Hi,
> 
> On Tue, Feb 5, 2013 at 8:43 AM, Minchan Kim <minchan@kernel.org> wrote:
> > Hello,
> >
> > On Tue, Feb 05, 2013 at 08:29:26AM +0900, Kyungmin Park wrote:
> >> On Tue, Feb 5, 2013 at 8:06 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> >> > On Mon, 04 Feb 2013 11:27:05 +0100
> >> > Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> >
> >> >> The total number of low memory pages is determined as
> >> >> totalram_pages - totalhigh_pages, so without this patch all CMA
> >> >> pageblocks placed in highmem were accounted to low memory.
> >> >
> >> > What are the end-user-visible effects of this bug?
> >>
> >> Even though CMA is located at highmem. LowTotal has more than lowmem
> >> address spaces.
> >>
> >> e.g.,
> >> lowmem  : 0xc0000000 - 0xdf000000   ( 496 MB)
> >> LowTotal:         555788 kB
> >>
> >> >
> >> > (This information is needed so that others can make patch-scheduling
> >> > decisions and should be included in all bugfix changelogs unless it is
> >> > obvious).
> >>
> >> CMA Highmem support is new feature. so don't need to go stable tree.
> >
> > I would like to clarify it because I remembered alloc_migrate_target have considered
> > CMA pages could be highmem. Is it really new feature? If so, could you point out
> > enabling patches for the new feature?
> >
> Here's related patch.
> http://www.spinics.net/lists/arm-kernel/msg222369.html

Thanks.

> 
> Previous time, it's not fully tested and now we checked it with
> highmem support patches.

I get it. Sigh. then [1] inline attached below wan't good.
We have to code like this?

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

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b97cf12..0707e0a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5671,11 +5671,10 @@ 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;
-
+       gfp_t gfp_mask = GFP_HIGHUSER_MOVABLE;
+       struct address_space *mapping = page_mapping(page);
+       if (mapping)
+               gfp_mask = mapping_gfp_mask(mapping);
        return alloc_page(gfp_mask);
 }


commit 6a6dccba2fdc2a69f1f36b8f1c0acc8598e7221b
Author: Rabin Vincent <rabin@rab.in>
Date:   Thu Jul 5 15:52:23 2012 +0530

    mm: cma: don't replace lowmem pages with highmem
    
    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)
    
    Fix this by replacing only highmem pages with highmem.
    
    Reported-by: Laura Abbott <lauraa@codeaurora.org>
    Signed-off-by: Rabin Vincent <rabin@rab.in>
    Acked-by: Michal Nazarewicz <mina86@mina86.com>
    Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4403009..4a4f921 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5635,7 +5635,12 @@ static struct page *
 __alloc_contig_migrate_alloc(struct page *page, unsigned long private,
                             int **resultp)
 {
-       return alloc_page(GFP_HIGHUSER_MOVABLE);
+       gfp_t gfp_mask = GFP_USER | __GFP_MOVABLE;
+
+       if (PageHighMem(page))
+               gfp_mask |= __GFP_HIGHMEM;
+
+       return alloc_page(gfp_mask);
 }

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:34 ` Minchan Kim
@ 2013-02-05  7:10   ` Marek Szyprowski
  2013-02-05  7:34     ` Minchan Kim
  2013-02-19 13:27     ` Simon Jeons
  0 siblings, 2 replies; 16+ messages in thread
From: Marek Szyprowski @ 2013-02-05  7:10 UTC (permalink / raw)
  To: Minchan Kim; +Cc: linux-mm, linux-kernel, akpm, mgorman, kyungmin.park

Hello,

On 2/5/2013 12:34 AM, Minchan Kim wrote:
> On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
> > The total number of low memory pages is determined as
> > totalram_pages - totalhigh_pages, so without this patch all CMA
> > pageblocks placed in highmem were accounted to low memory.
>
> So what's the end user effect? With the effect, we have to decide
> routing it on stable.
>
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >  mm/page_alloc.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index f5bab0a..6415d93 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct page *page)
> >  	set_pageblock_migratetype(page, MIGRATE_CMA);
> >  	__free_pages(page, pageblock_order);
> >  	totalram_pages += pageblock_nr_pages;
> > +#ifdef CONFIG_HIGHMEM
>
> We don't need #ifdef/#endif.

#ifdef is required to let this code compile when highmem is not enabled,
becuase totalhigh_pages is defined as 0, see include/linux/highmem.h

> > +	if (PageHighMem(page))
> > +		totalhigh_pages += pageblock_nr_pages;
> > +#endif
> >  }
> >  #endif
> >

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-05  7:10   ` Marek Szyprowski
@ 2013-02-05  7:34     ` Minchan Kim
  2013-02-19 13:27     ` Simon Jeons
  1 sibling, 0 replies; 16+ messages in thread
From: Minchan Kim @ 2013-02-05  7:34 UTC (permalink / raw)
  To: Marek Szyprowski; +Cc: linux-mm, linux-kernel, akpm, mgorman, kyungmin.park

Hi Marek,

On Tue, Feb 05, 2013 at 08:10:19AM +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 2/5/2013 12:34 AM, Minchan Kim wrote:
> >On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
> >> The total number of low memory pages is determined as
> >> totalram_pages - totalhigh_pages, so without this patch all CMA
> >> pageblocks placed in highmem were accounted to low memory.
> >
> >So what's the end user effect? With the effect, we have to decide
> >routing it on stable.
> >
> >>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >>  mm/page_alloc.c |    4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >> index f5bab0a..6415d93 100644
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct page *page)
> >>  	set_pageblock_migratetype(page, MIGRATE_CMA);
> >>  	__free_pages(page, pageblock_order);
> >>  	totalram_pages += pageblock_nr_pages;
> >> +#ifdef CONFIG_HIGHMEM
> >
> >We don't need #ifdef/#endif.
> 
> #ifdef is required to let this code compile when highmem is not enabled,
> becuase totalhigh_pages is defined as 0, see include/linux/highmem.h

Argh, Sorry for the noise.
I think it would be better to use accessor but it's a just nitpick. :)
Thanks!

> 
> >> +	if (PageHighMem(page))
> >> +		totalhigh_pages += pageblock_nr_pages;
> >> +#endif
> >>  }
> >>  #endif
> >>
> 
> Best regards
> -- 
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-04 23:29   ` Kyungmin Park
  2013-02-04 23:43     ` Minchan Kim
@ 2013-02-05  8:28     ` Mel Gorman
  2013-02-05  8:56       ` Marek Szyprowski
  1 sibling, 1 reply; 16+ messages in thread
From: Mel Gorman @ 2013-02-05  8:28 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Andrew Morton, Marek Szyprowski, linux-mm, linux-kernel, minchan

On Tue, Feb 05, 2013 at 08:29:26AM +0900, Kyungmin Park wrote:
> >
> > (This information is needed so that others can make patch-scheduling
> > decisions and should be included in all bugfix changelogs unless it is
> > obvious).
> 
> CMA Highmem support is new feature. so don't need to go stable tree.
> 

You could have given a lot more information to that question!

How new a feature is it? Does this mean that this patch must go in before
3.8 releases or is it a fix against a patch that is only in Andrew's tree?
If the patch is only in Andrew's tree, which one is it and should this be
folded in as a fix?

On a semi-related note; is there a plan for backporting highmem support for
the LTSI kernel considering it's aimed at embedded and CMA was highlighted
in their announcment for 3.4 support?

-- 
Mel Gorman
SUSE Labs

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-05  0:40         ` Minchan Kim
@ 2013-02-05  8:38           ` Marek Szyprowski
  2013-02-05  8:47             ` Minchan Kim
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Szyprowski @ 2013-02-05  8:38 UTC (permalink / raw)
  To: Minchan Kim; +Cc: Kyungmin Park, Andrew Morton, linux-mm, linux-kernel, mgorman

Hello,

On 2/5/2013 1:40 AM, Minchan Kim wrote:

...

> > Previous time, it's not fully tested and now we checked it with
> > highmem support patches.
>
> I get it. Sigh. then [1] inline attached below wan't good.
> We have to code like this?
>
> [1] 6a6dccba, mm: cma: don't replace lowmem pages with highmem
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b97cf12..0707e0a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5671,11 +5671,10 @@ 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;
> -
> +       gfp_t gfp_mask = GFP_HIGHUSER_MOVABLE;
> +       struct address_space *mapping = page_mapping(page);
> +       if (mapping)
> +               gfp_mask = mapping_gfp_mask(mapping);
>          return alloc_page(gfp_mask);
>   }

Am I right that this code will allocate more pages from himem? Old approach
never migrate lowmem page to himem, what is now possible as gfp mask is 
always
taken from mapping_gfp flags. I only wonder if forcing GFP_HIGHUSER_MOVABLE
for pages without the mapping is a correct. Shouldn't we use avoid himem in
such case?

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-05  8:38           ` Marek Szyprowski
@ 2013-02-05  8:47             ` Minchan Kim
  0 siblings, 0 replies; 16+ messages in thread
From: Minchan Kim @ 2013-02-05  8:47 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Kyungmin Park, Andrew Morton, linux-mm, linux-kernel, mgorman

On Tue, Feb 05, 2013 at 09:38:30AM +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 2/5/2013 1:40 AM, Minchan Kim wrote:
> 
> ...
> 
> >> Previous time, it's not fully tested and now we checked it with
> >> highmem support patches.
> >
> >I get it. Sigh. then [1] inline attached below wan't good.
> >We have to code like this?
> >
> >[1] 6a6dccba, mm: cma: don't replace lowmem pages with highmem
> >
> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >index b97cf12..0707e0a 100644
> >--- a/mm/page_alloc.c
> >+++ b/mm/page_alloc.c
> >@@ -5671,11 +5671,10 @@ 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;
> >-
> >+       gfp_t gfp_mask = GFP_HIGHUSER_MOVABLE;
> >+       struct address_space *mapping = page_mapping(page);
> >+       if (mapping)
> >+               gfp_mask = mapping_gfp_mask(mapping);
> >         return alloc_page(gfp_mask);
> >  }
> 
> Am I right that this code will allocate more pages from himem? Old approach

Yes.

> never migrate lowmem page to himem, what is now possible as gfp mask
> is always
> taken from mapping_gfp flags. I only wonder if forcing GFP_HIGHUSER_MOVABLE

-ENOPARSE. What is not possbile ~~ take from mapping_gfp flags.
Could you clarify your statement?

> for pages without the mapping is a correct. Shouldn't we use avoid himem in

CMA pages is for pages for user, NOT kernel so HIGHUSER_MOVABLE makes sense.

> such case?

I don't get it. :(
We have to recomment use of highmem for user space pages.
Am I missing something?

Sorry, I should go out of office now so forgive my late response.



> 
> Best regards
> -- 
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 
> --
> 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>

-- 
Kind regards,
Minchan Kim

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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-05  8:28     ` Mel Gorman
@ 2013-02-05  8:56       ` Marek Szyprowski
  0 siblings, 0 replies; 16+ messages in thread
From: Marek Szyprowski @ 2013-02-05  8:56 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Kyungmin Park, Andrew Morton, linux-mm, linux-kernel, minchan

Hello,

On 2/5/2013 9:28 AM, Mel Gorman wrote:
> On Tue, Feb 05, 2013 at 08:29:26AM +0900, Kyungmin Park wrote:
> > >
> > > (This information is needed so that others can make patch-scheduling
> > > decisions and should be included in all bugfix changelogs unless it is
> > > obvious).
> >
> > CMA Highmem support is new feature. so don't need to go stable tree.
> >
>
> You could have given a lot more information to that question!
>
> How new a feature is it?

ARM DMA-mapping, the only in-kernel client of CMA, will gain himem 
support in
v3.9. On the other hand, there might be out of tree clients of
alloc_contig_migrate_range()/dma_alloc_from_contiguous() API. If you think
we should care about them, then this patch might need to be backported
to stable kernels.

>   Does this mean that this patch must go in before
> 3.8 releases or is it a fix against a patch that is only in Andrew's tree?
> If the patch is only in Andrew's tree, which one is it and should this be
> folded in as a fix?
>
> On a semi-related note; is there a plan for backporting highmem support for
> the LTSI kernel considering it's aimed at embedded and CMA was highlighted
> in their announcment for 3.4 support?

I've just noticed recently that LTSI released v3.4 kernel with CMA support.
I've checked that code only briefly and noticed that it didn't have all the
CMA related patches which are available in v3.8-rc1. I will take a look at
that code and maybe I will find some time to backport some more patches from
mainline, but please note that mainline kernel has higher priority.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-05  7:10   ` Marek Szyprowski
  2013-02-05  7:34     ` Minchan Kim
@ 2013-02-19 13:27     ` Simon Jeons
  2013-02-20  2:57       ` Kyungmin Park
  1 sibling, 1 reply; 16+ messages in thread
From: Simon Jeons @ 2013-02-19 13:27 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Minchan Kim, linux-mm, linux-kernel, akpm, mgorman, kyungmin.park

On 02/05/2013 03:10 PM, Marek Szyprowski wrote:
> Hello,
>
> On 2/5/2013 12:34 AM, Minchan Kim wrote:
>> On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
>> > The total number of low memory pages is determined as
>> > totalram_pages - totalhigh_pages, so without this patch all CMA
>> > pageblocks placed in highmem were accounted to low memory.
>>
>> So what's the end user effect? With the effect, we have to decide
>> routing it on stable.
>>
>> >
>> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> > ---
>> >  mm/page_alloc.c |    4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> > index f5bab0a..6415d93 100644
>> > --- a/mm/page_alloc.c
>> > +++ b/mm/page_alloc.c
>> > @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct 
>> page *page)
>> >      set_pageblock_migratetype(page, MIGRATE_CMA);
>> >      __free_pages(page, pageblock_order);
>> >      totalram_pages += pageblock_nr_pages;
>> > +#ifdef CONFIG_HIGHMEM
>>
>> We don't need #ifdef/#endif.
>
> #ifdef is required to let this code compile when highmem is not enabled,
> becuase totalhigh_pages is defined as 0, see include/linux/highmem.h
>

Hi Marek,

1) Why can support CMA regions placed in highmem? CMA is for dma buffer, 
correct? Then how can old dma device access highmem?
2) Why there is no totalhigh_pages variable define in the case of config 
highmem?

>> > +    if (PageHighMem(page))
>> > +        totalhigh_pages += pageblock_nr_pages;
>> > +#endif
>> >  }
>> >  #endif
>> >
>
> Best regards


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

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-19 13:27     ` Simon Jeons
@ 2013-02-20  2:57       ` Kyungmin Park
  2013-02-20  5:31         ` Simon Jeons
  0 siblings, 1 reply; 16+ messages in thread
From: Kyungmin Park @ 2013-02-20  2:57 UTC (permalink / raw)
  To: Simon Jeons
  Cc: Marek Szyprowski, Minchan Kim, linux-mm, linux-kernel, akpm, mgorman

On Tue, Feb 19, 2013 at 10:27 PM, Simon Jeons <simon.jeons@gmail.com> wrote:
> On 02/05/2013 03:10 PM, Marek Szyprowski wrote:
>>
>> Hello,
>>
>> On 2/5/2013 12:34 AM, Minchan Kim wrote:
>>>
>>> On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
>>> > The total number of low memory pages is determined as
>>> > totalram_pages - totalhigh_pages, so without this patch all CMA
>>> > pageblocks placed in highmem were accounted to low memory.
>>>
>>> So what's the end user effect? With the effect, we have to decide
>>> routing it on stable.
>>>
>>> >
>>> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> > ---
>>> >  mm/page_alloc.c |    4 ++++
>>> >  1 file changed, 4 insertions(+)
>>> >
>>> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> > index f5bab0a..6415d93 100644
>>> > --- a/mm/page_alloc.c
>>> > +++ b/mm/page_alloc.c
>>> > @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct
>>> > page *page)
>>> >      set_pageblock_migratetype(page, MIGRATE_CMA);
>>> >      __free_pages(page, pageblock_order);
>>> >      totalram_pages += pageblock_nr_pages;
>>> > +#ifdef CONFIG_HIGHMEM
>>>
>>> We don't need #ifdef/#endif.
>>
>>
>> #ifdef is required to let this code compile when highmem is not enabled,
>> becuase totalhigh_pages is defined as 0, see include/linux/highmem.h
>>
>
> Hi Marek,
>
> 1) Why can support CMA regions placed in highmem?
Some vendors use reserved memory at highmem, and it's hard to modify
to use lowmem, so just CMA can support highmem and no need to adjust
address used at reserved memory.
> CMA is for dma buffer, correct? Then how can old dma device access highmem?
What's the "old dma device"? To support it, we also modify
arch/arm/mm/dma-mapping.c for handling highmem address.

> 2) Why there is no totalhigh_pages variable define in the case of config
> highmem?
I don't know. it's just defined in case of highmem. that's reason to
use #ifdef/endif. we don't know hisotrical reason.

Thank you,
Kyungmin Park
>
>
>>> > +    if (PageHighMem(page))
>>> > +        totalhigh_pages += pageblock_nr_pages;
>>> > +#endif
>>> >  }
>>> >  #endif
>>> >
>>
>>
>> Best regards
>
>
> --
> 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] 16+ messages in thread

* Re: [PATCH] mm: cma: fix accounting of CMA pages placed in high memory
  2013-02-20  2:57       ` Kyungmin Park
@ 2013-02-20  5:31         ` Simon Jeons
  0 siblings, 0 replies; 16+ messages in thread
From: Simon Jeons @ 2013-02-20  5:31 UTC (permalink / raw)
  To: Kyungmin Park
  Cc: Marek Szyprowski, Minchan Kim, linux-mm, linux-kernel, akpm, mgorman

On 02/20/2013 10:57 AM, Kyungmin Park wrote:
> On Tue, Feb 19, 2013 at 10:27 PM, Simon Jeons <simon.jeons@gmail.com> wrote:
>> On 02/05/2013 03:10 PM, Marek Szyprowski wrote:
>>> Hello,
>>>
>>> On 2/5/2013 12:34 AM, Minchan Kim wrote:
>>>> On Mon, Feb 04, 2013 at 11:27:05AM +0100, Marek Szyprowski wrote:
>>>>> The total number of low memory pages is determined as
>>>>> totalram_pages - totalhigh_pages, so without this patch all CMA
>>>>> pageblocks placed in highmem were accounted to low memory.
>>>> So what's the end user effect? With the effect, we have to decide
>>>> routing it on stable.
>>>>
>>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> ---
>>>>>   mm/page_alloc.c |    4 ++++
>>>>>   1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>>> index f5bab0a..6415d93 100644
>>>>> --- a/mm/page_alloc.c
>>>>> +++ b/mm/page_alloc.c
>>>>> @@ -773,6 +773,10 @@ void __init init_cma_reserved_pageblock(struct
>>>>> page *page)
>>>>>       set_pageblock_migratetype(page, MIGRATE_CMA);
>>>>>       __free_pages(page, pageblock_order);
>>>>>       totalram_pages += pageblock_nr_pages;
>>>>> +#ifdef CONFIG_HIGHMEM
>>>> We don't need #ifdef/#endif.
>>>
>>> #ifdef is required to let this code compile when highmem is not enabled,
>>> becuase totalhigh_pages is defined as 0, see include/linux/highmem.h
>>>
>> Hi Marek,
>>
>> 1) Why can support CMA regions placed in highmem?
> Some vendors use reserved memory at highmem, and it's hard to modify
> to use lowmem, so just CMA can support highmem and no need to adjust
> address used at reserved memory.
>> CMA is for dma buffer, correct? Then how can old dma device access highmem?
> What's the "old dma device"? To support it, we also modify

I mean dma devices which should still use bounce buffer, then these 
devices will use bounce buffer to access CMA region in highmem or ...?

> arch/arm/mm/dma-mapping.c for handling highmem address.
>
>> 2) Why there is no totalhigh_pages variable define in the case of config
>> highmem?
> I don't know. it's just defined in case of highmem. that's reason to
> use #ifdef/endif. we don't know hisotrical reason.
>
> Thank you,
> Kyungmin Park
>>
>>>>> +    if (PageHighMem(page))
>>>>> +        totalhigh_pages += pageblock_nr_pages;
>>>>> +#endif
>>>>>   }
>>>>>   #endif
>>>>>
>>>
>>> Best regards
>>
>> --
>> 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] 16+ messages in thread

end of thread, other threads:[~2013-02-20  5:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-04 10:27 [PATCH] mm: cma: fix accounting of CMA pages placed in high memory Marek Szyprowski
2013-02-04 23:06 ` Andrew Morton
2013-02-04 23:29   ` Kyungmin Park
2013-02-04 23:43     ` Minchan Kim
2013-02-04 23:52       ` Kyungmin Park
2013-02-05  0:40         ` Minchan Kim
2013-02-05  8:38           ` Marek Szyprowski
2013-02-05  8:47             ` Minchan Kim
2013-02-05  8:28     ` Mel Gorman
2013-02-05  8:56       ` Marek Szyprowski
2013-02-04 23:34 ` Minchan Kim
2013-02-05  7:10   ` Marek Szyprowski
2013-02-05  7:34     ` Minchan Kim
2013-02-19 13:27     ` Simon Jeons
2013-02-20  2:57       ` Kyungmin Park
2013-02-20  5:31         ` Simon Jeons

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).