[PATCH] mm: fix page_owner initializing issue for arm32

[PATCH] mm: fix page_owner initializing issue for arm32

[Zhenhua Huang]

Page owner of pages used by page owner itself used is missing on arm32 targets.
The reason is dummy_handle and failure_handle is not initialized correctly.
Buddy allocator is used to initialize these two handles. However, buddy
allocator is not ready when page owner calls it. This change fixed that by
initializing page owner after buddy initialization.

The working flow before and after this change are:
original logic:
1. allocated memory for page_ext(using memblock).
2. invoke the init callback of page_ext_ops like
page_owner(using buddy allocator).
3. initialize buddy.

after this change:
1. allocated memory for page_ext(using memblock).
2. initialize buddy.
3. invoke the init callback of page_ext_ops like
page_owner(using buddy allocator).

with the change, failure/dummy_handle can get its correct value and
page owner output for example has the one for page owner itself:
Page allocated via order 2, mask 0x6202c0(GFP_USER|__GFP_NOWARN), pid 1006, ts
67278156558 ns
PFN 543776 type Unmovable Block 531 type Unmovable Flags 0x0()
 init_page_owner+0x28/0x2f8
 invoke_init_callbacks_flatmem+0x24/0x34
 start_kernel+0x33c/0x5d8
   (null)

Signed-off-by: Zhenhua Huang <zhenhuah@codeaurora.org>
---
 include/linux/page_ext.h | 8 ++++++++
 init/main.c              | 2 ++
 mm/page_ext.c            | 8 +++++++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index cfce186..aff81ba 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -44,8 +44,12 @@ static inline void page_ext_init_flatmem(void)
 {
 }
 extern void page_ext_init(void);
+static inline void page_ext_init_flatmem_late(void)
+{
+}
 #else
 extern void page_ext_init_flatmem(void);
+extern void page_ext_init_flatmem_late(void);
 static inline void page_ext_init(void)
 {
 }
@@ -76,6 +80,10 @@ static inline void page_ext_init(void)
 {
 }
 
+static inline void page_ext_init_flatmem_late(void)
+{
+}
+
 static inline void page_ext_init_flatmem(void)
 {
 }
diff --git a/init/main.c b/init/main.c
index 130376e..b34c475 100644
--- a/init/main.c
+++ b/init/main.c
@@ -818,6 +818,8 @@ static void __init mm_init(void)
 	init_debug_pagealloc();
 	report_meminit();
 	mem_init();
+	/* page_owner must be initialized after buddy is ready */
+	page_ext_init_flatmem_late();
 	kmem_cache_init();
 	kmemleak_init();
 	pgtable_init();
diff --git a/mm/page_ext.c b/mm/page_ext.c
index a3616f7..373f7a1 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -99,6 +99,13 @@ static void __init invoke_init_callbacks(void)
 	}
 }
 
+#if !defined(CONFIG_SPARSEMEM)
+void __init page_ext_init_flatmem_late(void)
+{
+	invoke_init_callbacks();
+}
+#endif
+
 static inline struct page_ext *get_entry(void *base, unsigned long index)
 {
 	return base + page_ext_size * index;
@@ -177,7 +184,6 @@ void __init page_ext_init_flatmem(void)
 			goto fail;
 	}
 	pr_info("allocated %ld bytes of page_ext\n", total_usage);
-	invoke_init_callbacks();
 	return;
 
 fail:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] mm: fix page_owner initializing issue for arm32

[Vlastimil Babka]

On 10/16/20 11:14 AM, Zhenhua Huang wrote:
> Page owner of pages used by page owner itself used is missing on arm32 targets.
> The reason is dummy_handle and failure_handle is not initialized correctly.
> Buddy allocator is used to initialize these two handles. However, buddy
> allocator is not ready when page owner calls it. This change fixed that by
> initializing page owner after buddy initialization.
> 
> The working flow before and after this change are:
> original logic:
> 1. allocated memory for page_ext(using memblock).
> 2. invoke the init callback of page_ext_ops like
> page_owner(using buddy allocator).
> 3. initialize buddy.
> 
> after this change:
> 1. allocated memory for page_ext(using memblock).
> 2. initialize buddy.
> 3. invoke the init callback of page_ext_ops like
> page_owner(using buddy allocator).
> 
> with the change, failure/dummy_handle can get its correct value and
> page owner output for example has the one for page owner itself:
> Page allocated via order 2, mask 0x6202c0(GFP_USER|__GFP_NOWARN), pid 1006, ts
> 67278156558 ns
> PFN 543776 type Unmovable Block 531 type Unmovable Flags 0x0()
>   init_page_owner+0x28/0x2f8
>   invoke_init_callbacks_flatmem+0x24/0x34
>   start_kernel+0x33c/0x5d8
>     (null)

register_dummy_stack should also appear in the above. Either one too many is 
skipped in arm32 stack saving, or the noinline is not honoured. Could be 
investigated separately.

> 
> Signed-off-by: Zhenhua Huang <zhenhuah@codeaurora.org>

This should be safe, as the sparse variant page_ext_init() runs even later, so:

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Nit below:

> ---
>   include/linux/page_ext.h | 8 ++++++++
>   init/main.c              | 2 ++
>   mm/page_ext.c            | 8 +++++++-
>   3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index cfce186..aff81ba 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -44,8 +44,12 @@ static inline void page_ext_init_flatmem(void)
>   {
>   }
>   extern void page_ext_init(void);
> +static inline void page_ext_init_flatmem_late(void)
> +{
> +}
>   #else
>   extern void page_ext_init_flatmem(void);
> +extern void page_ext_init_flatmem_late(void);
>   static inline void page_ext_init(void)
>   {
>   }
> @@ -76,6 +80,10 @@ static inline void page_ext_init(void)
>   {
>   }
>   
> +static inline void page_ext_init_flatmem_late(void)
> +{
> +}
> +
>   static inline void page_ext_init_flatmem(void)
>   {
>   }
> diff --git a/init/main.c b/init/main.c
> index 130376e..b34c475 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -818,6 +818,8 @@ static void __init mm_init(void)
>   	init_debug_pagealloc();
>   	report_meminit();
>   	mem_init();
> +	/* page_owner must be initialized after buddy is ready */
> +	page_ext_init_flatmem_late();
>   	kmem_cache_init();
>   	kmemleak_init();
>   	pgtable_init();
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index a3616f7..373f7a1 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -99,6 +99,13 @@ static void __init invoke_init_callbacks(void)
>   	}
>   }
>   
> +#if !defined(CONFIG_SPARSEMEM)

#ifndef is more common if you don't need boolean ops on multiple configs

> +void __init page_ext_init_flatmem_late(void)
> +{
> +	invoke_init_callbacks();
> +}
> +#endif
> +
>   static inline struct page_ext *get_entry(void *base, unsigned long index)
>   {
>   	return base + page_ext_size * index;
> @@ -177,7 +184,6 @@ void __init page_ext_init_flatmem(void)
>   			goto fail;
>   	}
>   	pr_info("allocated %ld bytes of page_ext\n", total_usage);
> -	invoke_init_callbacks();
>   	return;
>   
>   fail:
> 

Re: [PATCH] mm: fix page_owner initializing issue for arm32

[Zhenhua Huang]

Fri, Oct 16, 2020 at 06:41:04PM +0800, Vlastimil Babka wrote:
> On 10/16/20 11:14 AM, Zhenhua Huang wrote:
> >Page owner of pages used by page owner itself used is missing on arm32
> >targets.
> >The reason is dummy_handle and failure_handle is not initialized
> >correctly.
> >Buddy allocator is used to initialize these two handles. However, buddy
> >allocator is not ready when page owner calls it. This change fixed that by
> >initializing page owner after buddy initialization.
> >
> >The working flow before and after this change are:
> >original logic:
> >1. allocated memory for page_ext(using memblock).
> >2. invoke the init callback of page_ext_ops like
> >page_owner(using buddy allocator).
> >3. initialize buddy.
> >
> >after this change:
> >1. allocated memory for page_ext(using memblock).
> >2. initialize buddy.
> >3. invoke the init callback of page_ext_ops like
> >page_owner(using buddy allocator).
> >
> >with the change, failure/dummy_handle can get its correct value and
> >page owner output for example has the one for page owner itself:
> >Page allocated via order 2, mask 0x6202c0(GFP_USER|__GFP_NOWARN), pid
> >1006, ts
> >67278156558 ns
> >PFN 543776 type Unmovable Block 531 type Unmovable Flags 0x0()
> >  init_page_owner+0x28/0x2f8
> >  invoke_init_callbacks_flatmem+0x24/0x34
> >  start_kernel+0x33c/0x5d8
> >    (null)
> 
> register_dummy_stack should also appear in the above. Either one too many is
> skipped in arm32 stack saving, or the noinline is not honoured. Could be
> investigated separately.
> 
yes, it's another issue I need to investigate. Be noted, the printing is from
4.9 kernel and I will look into separately.
> >
> >Signed-off-by: Zhenhua Huang <zhenhuah@codeaurora.org>
> 
> This should be safe, as the sparse variant page_ext_init() runs even later,
> so:
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Nit below:
> 
> >---
> >  include/linux/page_ext.h | 8 ++++++++
> >  init/main.c              | 2 ++
> >  mm/page_ext.c            | 8 +++++++-
> >  3 files changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> >index cfce186..aff81ba 100644
> >--- a/include/linux/page_ext.h
> >+++ b/include/linux/page_ext.h
> >@@ -44,8 +44,12 @@ static inline void page_ext_init_flatmem(void)
> >  {
> >  }
> >  extern void page_ext_init(void);
> >+static inline void page_ext_init_flatmem_late(void)
> >+{
> >+}
> >  #else
> >  extern void page_ext_init_flatmem(void);
> >+extern void page_ext_init_flatmem_late(void);
> >  static inline void page_ext_init(void)
> >  {
> >  }
> >@@ -76,6 +80,10 @@ static inline void page_ext_init(void)
> >  {
> >  }
> >
> >+static inline void page_ext_init_flatmem_late(void)
> >+{
> >+}
> >+
> >  static inline void page_ext_init_flatmem(void)
> >  {
> >  }
> >diff --git a/init/main.c b/init/main.c
> >index 130376e..b34c475 100644
> >--- a/init/main.c
> >+++ b/init/main.c
> >@@ -818,6 +818,8 @@ static void __init mm_init(void)
> >  	init_debug_pagealloc();
> >  	report_meminit();
> >  	mem_init();
> >+	/* page_owner must be initialized after buddy is ready */
> >+	page_ext_init_flatmem_late();
> >  	kmem_cache_init();
> >  	kmemleak_init();
> >  	pgtable_init();
> >diff --git a/mm/page_ext.c b/mm/page_ext.c
> >index a3616f7..373f7a1 100644
> >--- a/mm/page_ext.c
> >+++ b/mm/page_ext.c
> >@@ -99,6 +99,13 @@ static void __init invoke_init_callbacks(void)
> >  	}
> >  }
> >
> >+#if !defined(CONFIG_SPARSEMEM)
> 
> #ifndef is more common if you don't need boolean ops on multiple configs
> 
Thanks, I'll do the change in patchset v2
> >+void __init page_ext_init_flatmem_late(void)
> >+{
> >+	invoke_init_callbacks();
> >+}
> >+#endif
> >+
> >  static inline struct page_ext *get_entry(void *base, unsigned long
> >index)
> >  {
> >  	return base + page_ext_size * index;
> >@@ -177,7 +184,6 @@ void __init page_ext_init_flatmem(void)
> >  			goto fail;
> >  	}
> >  	pr_info("allocated %ld bytes of page_ext\n", total_usage);
> >-	invoke_init_callbacks();
> >  	return;
> >
> >  fail:
> >
> 
> 

Re: [PATCH] mm: fix page_owner initializing issue for arm32

[Mike Rapoport]

On Fri, Oct 16, 2020 at 05:14:00PM +0800, Zhenhua Huang wrote:
> Page owner of pages used by page owner itself used is missing on arm32 targets.
> The reason is dummy_handle and failure_handle is not initialized correctly.
> Buddy allocator is used to initialize these two handles. However, buddy
> allocator is not ready when page owner calls it. This change fixed that by
> initializing page owner after buddy initialization.
> 
> The working flow before and after this change are:
> original logic:
> 1. allocated memory for page_ext(using memblock).

Is anything that requires a memblock allocation FLATMEM?
Any fundamental reason why wouldn't alloc_pages_exact_nid/vzalloc_node()
work in this case?

It seems to me that for FLATMEM configuration we can allocate the
page_ext using alloc_pages() with a fallback to vzalloc_node() and then
we can unify lot's of page_ext code and entirely drop
page_ext_init_flatmem().

> 2. invoke the init callback of page_ext_ops like
> page_owner(using buddy allocator).
> 3. initialize buddy.
> 
> after this change:
> 1. allocated memory for page_ext(using memblock).
> 2. initialize buddy.
> 3. invoke the init callback of page_ext_ops like
> page_owner(using buddy allocator).
> 
> with the change, failure/dummy_handle can get its correct value and
> page owner output for example has the one for page owner itself:
> Page allocated via order 2, mask 0x6202c0(GFP_USER|__GFP_NOWARN), pid 1006, ts
> 67278156558 ns
> PFN 543776 type Unmovable Block 531 type Unmovable Flags 0x0()
>  init_page_owner+0x28/0x2f8
>  invoke_init_callbacks_flatmem+0x24/0x34
>  start_kernel+0x33c/0x5d8
>    (null)
> 
> Signed-off-by: Zhenhua Huang <zhenhuah@codeaurora.org>
> ---
>  include/linux/page_ext.h | 8 ++++++++
>  init/main.c              | 2 ++
>  mm/page_ext.c            | 8 +++++++-
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index cfce186..aff81ba 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -44,8 +44,12 @@ static inline void page_ext_init_flatmem(void)
>  {
>  }
>  extern void page_ext_init(void);
> +static inline void page_ext_init_flatmem_late(void)
> +{
> +}
>  #else
>  extern void page_ext_init_flatmem(void);
> +extern void page_ext_init_flatmem_late(void);
>  static inline void page_ext_init(void)
>  {
>  }
> @@ -76,6 +80,10 @@ static inline void page_ext_init(void)
>  {
>  }
>  
> +static inline void page_ext_init_flatmem_late(void)
> +{
> +}
> +
>  static inline void page_ext_init_flatmem(void)
>  {
>  }
> diff --git a/init/main.c b/init/main.c
> index 130376e..b34c475 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -818,6 +818,8 @@ static void __init mm_init(void)
>  	init_debug_pagealloc();
>  	report_meminit();
>  	mem_init();
> +	/* page_owner must be initialized after buddy is ready */
> +	page_ext_init_flatmem_late();
>  	kmem_cache_init();
>  	kmemleak_init();
>  	pgtable_init();
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index a3616f7..373f7a1 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -99,6 +99,13 @@ static void __init invoke_init_callbacks(void)
>  	}
>  }
>  
> +#if !defined(CONFIG_SPARSEMEM)
> +void __init page_ext_init_flatmem_late(void)
> +{
> +	invoke_init_callbacks();
> +}
> +#endif
> +
>  static inline struct page_ext *get_entry(void *base, unsigned long index)
>  {
>  	return base + page_ext_size * index;
> @@ -177,7 +184,6 @@ void __init page_ext_init_flatmem(void)
>  			goto fail;
>  	}
>  	pr_info("allocated %ld bytes of page_ext\n", total_usage);
> -	invoke_init_callbacks();
>  	return;
>  
>  fail:
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH] mm: fix page_owner initializing issue for arm32

[Mike Rapoport]

On Mon, Oct 26, 2020 at 03:12:55PM +0800, Zhenhua Huang wrote:
> Hi Mike,
> 
> On Sun, Oct 25, 2020 at 11:42:53PM +0800, Mike Rapoport wrote:
> > On Fri, Oct 16, 2020 at 05:14:00PM +0800, Zhenhua Huang wrote:
> > > Page owner of pages used by page owner itself used is missing on arm32
> > targets.
> > > The reason is dummy_handle and failure_handle is not initialized
> > correctly.
> > > Buddy allocator is used to initialize these two handles. However, buddy
> > > allocator is not ready when page owner calls it. This change fixed that
> > by
> > > initializing page owner after buddy initialization.
> > > 
> > > The working flow before and after this change are:
> > > original logic:
> > > 1. allocated memory for page_ext(using memblock).
> > 
> > Is anything that requires a memblock allocation FLATMEM?
> > Any fundamental reason why wouldn't alloc_pages_exact_nid/vzalloc_node()
> > work in this case?
> > 
> > It seems to me that for FLATMEM configuration we can allocate the
> > page_ext using alloc_pages() with a fallback to vzalloc_node() and then
> > we can unify lot's of page_ext code and entirely drop
> > page_ext_init_flatmem().
>
> From comments in codes: "page_ext requires contiguous pages, bigger than
> MAX_ORDER unless SPARSEMEM."
> The size of page_ext for FLATMEM(which used pgdat) should be much larger than
> the size for SPARSEMEM which used section.

Well, the vzalloc_node() fallback in alloc_page_ext() for SPARSEMEM case
implies that using pages that are not physically contiguous should be
fine. So, it seems to me that the comment is stale and vzalloc_node()
would work just fine for FLATMEM case if the allocation of page_ext
would exceed MAX_ORDER.

> > > 2. invoke the init callback of page_ext_ops like
> > > page_owner(using buddy allocator).
> > > 3. initialize buddy.
> > > 
> > > after this change:
> > > 1. allocated memory for page_ext(using memblock).
> > > 2. initialize buddy.
> > > 3. invoke the init callback of page_ext_ops like
> > > page_owner(using buddy allocator).
> > > 
> > > with the change, failure/dummy_handle can get its correct value and
> > > page owner output for example has the one for page owner itself:
> > > Page allocated via order 2, mask 0x6202c0(GFP_USER|__GFP_NOWARN), pid
> > 1006, ts
> > > 67278156558 ns
> > > PFN 543776 type Unmovable Block 531 type Unmovable Flags 0x0()
> > >  init_page_owner+0x28/0x2f8
> > >  invoke_init_callbacks_flatmem+0x24/0x34
> > >  start_kernel+0x33c/0x5d8
> > >    (null)
> > > 
> > > Signed-off-by: Zhenhua Huang <zhenhuah@codeaurora.org>
> > > ---
> > >  include/linux/page_ext.h | 8 ++++++++
> > >  init/main.c              | 2 ++
> > >  mm/page_ext.c            | 8 +++++++-
> > >  3 files changed, 17 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> > > index cfce186..aff81ba 100644
> > > --- a/include/linux/page_ext.h
> > > +++ b/include/linux/page_ext.h
> > > @@ -44,8 +44,12 @@ static inline void page_ext_init_flatmem(void)
> > >  {
> > >  }
> > >  extern void page_ext_init(void);
> > > +static inline void page_ext_init_flatmem_late(void)
> > > +{
> > > +}
> > >  #else
> > >  extern void page_ext_init_flatmem(void);
> > > +extern void page_ext_init_flatmem_late(void);
> > >  static inline void page_ext_init(void)
> > >  {
> > >  }
> > > @@ -76,6 +80,10 @@ static inline void page_ext_init(void)
> > >  {
> > >  }
> > >  
> > > +static inline void page_ext_init_flatmem_late(void)
> > > +{
> > > +}
> > > +
> > >  static inline void page_ext_init_flatmem(void)
> > >  {
> > >  }
> > > diff --git a/init/main.c b/init/main.c
> > > index 130376e..b34c475 100644
> > > --- a/init/main.c
> > > +++ b/init/main.c
> > > @@ -818,6 +818,8 @@ static void __init mm_init(void)
> > >  	init_debug_pagealloc();
> > >  	report_meminit();
> > >  	mem_init();
> > > +	/* page_owner must be initialized after buddy is ready */
> > > +	page_ext_init_flatmem_late();
> > >  	kmem_cache_init();
> > >  	kmemleak_init();
> > >  	pgtable_init();
> > > diff --git a/mm/page_ext.c b/mm/page_ext.c
> > > index a3616f7..373f7a1 100644
> > > --- a/mm/page_ext.c
> > > +++ b/mm/page_ext.c
> > > @@ -99,6 +99,13 @@ static void __init invoke_init_callbacks(void)
> > >  	}
> > >  }
> > >  
> > > +#if !defined(CONFIG_SPARSEMEM)
> > > +void __init page_ext_init_flatmem_late(void)
> > > +{
> > > +	invoke_init_callbacks();
> > > +}
> > > +#endif
> > > +
> > >  static inline struct page_ext *get_entry(void *base, unsigned long
> > index)
> > >  {
> > >  	return base + page_ext_size * index;
> > > @@ -177,7 +184,6 @@ void __init page_ext_init_flatmem(void)
> > >  			goto fail;
> > >  	}
> > >  	pr_info("allocated %ld bytes of page_ext\n", total_usage);
> > > -	invoke_init_callbacks();
> > >  	return;
> > >  
> > >  fail:
> > > -- 
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > > 
> > > 
> > 
> > -- 
> > Sincerely yours,
> > Mike.
> > 

-- 
Sincerely yours,
Mike.