[PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Daniel Vacek]

On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
causes a boot hang. This patch fixes the hang making sure the alignment
never steps back.

Link: http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
Signed-off-by: Daniel Vacek <neelx@redhat.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: <stable@vger.kernel.org>
---
 mm/page_alloc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3d974cb2a1a1..e033a6895c6f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 			 * is not. move_freepages_block() can shift ahead of
 			 * the valid region but still depends on correct page
 			 * metadata.
+			 * Also make sure we never step back.
 			 */
-			pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
+			unsigned long next_pfn;
+
+			next_pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
 					~(pageblock_nr_pages-1)) - 1;
+			if (next_pfn > pfn)
+				pfn = next_pfn;
 #endif
 			continue;
 		}
-- 
2.16.2

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Michal Hocko]

On Tue 13-03-18 23:42:40, Daniel Vacek wrote:
> On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
> causes a boot hang. This patch fixes the hang making sure the alignment
> never steps back.

I am sorry to be complaining again, but the code is so obscure that I
would _really_ appreciate some more information about what is going
on here. memblock_next_valid_pfn will most likely return a pfn within
the same memblock and the alignment will move it before the old pfn
which is not valid - so the block has some holes. Is that correct?
If yes then please put it into the changelog. Maybe reuse data provided
by Arnd http://lkml.kernel.org/r/20180314134431.13241-1-ard.biesheuvel@linaro.org
 
> Link: http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
> Signed-off-by: Daniel Vacek <neelx@redhat.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: <stable@vger.kernel.org>
> ---
>  mm/page_alloc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3d974cb2a1a1..e033a6895c6f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  			 * is not. move_freepages_block() can shift ahead of
>  			 * the valid region but still depends on correct page
>  			 * metadata.
> +			 * Also make sure we never step back.
>  			 */
> -			pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
> +			unsigned long next_pfn;
> +
> +			next_pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>  					~(pageblock_nr_pages-1)) - 1;
> +			if (next_pfn > pfn)
> +				pfn = next_pfn;
>  #endif
>  			continue;
>  		}
> -- 
> 2.16.2
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Daniel Vacek]

On Wed, Mar 14, 2018 at 3:17 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 13-03-18 23:42:40, Daniel Vacek wrote:
>> On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>> causes a boot hang. This patch fixes the hang making sure the alignment
>> never steps back.
>
> I am sorry to be complaining again, but the code is so obscure that I

No worries, I'm glad for any review. Which code exactly you do find
obscure? This patch or my former fix or the original commit
introducing memblock_next_valid_pfn()? Coz I'd agree the original
commit looks pretty obscure...

> would _really_ appreciate some more information about what is going
> on here. memblock_next_valid_pfn will most likely return a pfn within
> the same memblock and the alignment will move it before the old pfn
> which is not valid - so the block has some holes. Is that correct?

I do not understand what you mean by 'pfn within the same memblock'?
And by 'the block has some holes'?

memblock has types 'memory' (as usable memory) and 'reserved' (for
unusable mem), if I understand correctly. Both of them have an array
of regions (as an equivalent or kind of abstraction of ranges, IIUC).
memblock is a global symbol which contains all of this. So one could
say all pfns are within the same memblock. Did you mean the same
memory region perhaps? A region is solid by definition, I believe. So
there should be no holes within a single region _by_definition_
anyways. Or am I wrong here?

Now, memblock_next_valid_pfn(pfn) is called only conditionally when
the old pfn is already 'invalid' in the first place. Here the meaning
of this 'invalid' is arch/config dependent. With my former fix based
on x86_64 config I was working with assumption that the semantics of
'invalid' means the memsection does not have memmap:

early_pfn_valid(pfn)
  pfn_valid(pfn)
    valid_section(__nr_to_section(pfn_to_section_nr(pfn)))
      return (section && (section->section_mem_map & SECTION_HAS_MEM_MAP))

>From this I implied that when memblock_next_valid_pfn(pfn) is called
this pfn is the first in a section (ie. section aligned) and this
whole section is going to be skipped. So that the returned pfn will
gonna be at least one full section away from the old pfn. That's also
exactly what I can see in the memory dumps from bugreports resulting
in my former fix.
Note that with next_pfn being at least a full section away from old
pfn, there is no need to check whether it steps back or not. Even when
pageblock aligned. That's why I did not include the hunk in this patch
in my former fix.

Now I have no idea why above said does not hold true for arm64 or what
config was used there. I did not have a chance nor time to get my
hands on any arm hardware where it broke. The same way as I did not
test any architecture but x86_64 where I had original reports of
crashes before applying my former fix.
Also I am not deeply experienced with mm details and internals and how
everything works on every architecture. And even less when we speak
about early boot init. I mostly only considered the algorithm
abstractions. I bisected and reviewed the patch which regressed
x86_64. It clearly skips some pfns based on memblock memory ranges (if
available) and the skipped pages are not initialized, causing the
crash. This is clearly visible in the memory dumps I got from reports
and hopefully I already clearly explained that. I fixed this by
applying the alignment to keep at least the bare minimum of pages
initialized (so skipping a bit less than to the beginning of the next
usable range in case that range is not
section/memblock/pageblock/whatever aligned - with the pageblock
alignment being the significant one here). Since the next pfn will
always be from different section on x86_64 (at least with the
configurations I checked and my limited knowledge of this stuff), this
patch was not originally applied as it seemed redundant to me at that
time. It was only theoretically possible the algo can start looping if
the next pfn would fall under the original one but it seemed this is
impossible to happen. Since this is generic code and not arch
specific, I expected other arches to behave in a similar manner.
Though it seems arm has it's own view of pfn_valid() based on memblock
instead of mem sections:

neelx@metal:~/nX/src/linux$ global -g CONFIG_HAVE_ARCH_PFN_VALID
arch/arm/include/asm/page.h
arch/arm/mm/init.c
arch/arm64/include/asm/page.h
arch/arm64/mm/init.c
include/linux/mmzone.h
neelx@metal:~/nX/src/linux$ sed '287,293!d' arch/arm64/mm/init.c
#ifdef CONFIG_HAVE_ARCH_PFN_VALID
int pfn_valid(unsigned long pfn)
{
    return memblock_is_map_memory(pfn << PAGE_SHIFT);
}
EXPORT_SYMBOL(pfn_valid);
#endif
neelx@metal:~/nX/src/linux$


ARM seems to be a bit unique here and that's what I missed and that's
why my former fix broke arm. I was simply not expecting this. And
that's why this patch is needed (exclusively for arm it seems).

Now we can start discussing if pfn_valid() based on mem sections is
fundamentally broken or if pfn_valid() on arm based on memblock when
CONFIG_HAVE_ARCH_PFN_VALID is enabled is broken. Personally I do not
like the double meaning of it.


Ard, Naresh, Sudeep> Is CONFIG_HAVE_ARCH_PFN_VALID actually enabled
for your builds?

--nX

> If yes then please put it into the changelog. Maybe reuse data provided
> by Arnd http://lkml.kernel.org/r/20180314134431.13241-1-ard.biesheuvel@linaro.org
>
>> Link: http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
>> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>> Signed-off-by: Daniel Vacek <neelx@redhat.com>
>> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Paul Burton <paul.burton@imgtec.com>
>> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: <stable@vger.kernel.org>
>> ---
>>  mm/page_alloc.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 3d974cb2a1a1..e033a6895c6f 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>>                        * is not. move_freepages_block() can shift ahead of
>>                        * the valid region but still depends on correct page
>>                        * metadata.
>> +                      * Also make sure we never step back.
>>                        */
>> -                     pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>> +                     unsigned long next_pfn;
>> +
>> +                     next_pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>>                                       ~(pageblock_nr_pages-1)) - 1;
>> +                     if (next_pfn > pfn)
>> +                             pfn = next_pfn;
>>  #endif
>>                       continue;
>>               }
>> --
>> 2.16.2
>>
>
> --
> Michal Hocko
> SUSE Labs

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Michal Hocko]

On Thu 15-03-18 02:30:41, Daniel Vacek wrote:
> On Wed, Mar 14, 2018 at 3:17 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Tue 13-03-18 23:42:40, Daniel Vacek wrote:
> >> On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
> >> causes a boot hang. This patch fixes the hang making sure the alignment
> >> never steps back.
> >
> > I am sorry to be complaining again, but the code is so obscure that I
> 
> No worries, I'm glad for any review. Which code exactly you do find
> obscure? This patch or my former fix or the original commit
> introducing memblock_next_valid_pfn()? Coz I'd agree the original
> commit looks pretty obscure...

As mentioned in the other email, the whole going back and forth in the
same loop is just too ugly to live.

> > would _really_ appreciate some more information about what is going
> > on here. memblock_next_valid_pfn will most likely return a pfn within
> > the same memblock and the alignment will move it before the old pfn
> > which is not valid - so the block has some holes. Is that correct?
> 
> I do not understand what you mean by 'pfn within the same memblock'?

Sorry, I should have said in the same pageblock

> And by 'the block has some holes'?

memblock_next_valid_pfn clearly returns pfn which is within a pageblock
and that is why we do not initialize pages in the begining of the block
while move_freepages_block does really expect the full pageblock to be
initialized properly. That is the fundamental problem, right?

> memblock has types 'memory' (as usable memory) and 'reserved' (for
> unusable mem), if I understand correctly.

We might not have struct pages for invalid pfns. That really depends on
the memory mode. Sure sparse mem model will usually allocate struct
pages for whole memory sections but that is not universally true and
adding such a suble assumption is simply wrong.

I suspect you are making strong assumptions based on a very specific
implementation which might be not true in general. That was the feeling
I've had since the patch was proposed for the first time. This is such a
cluttered area that I am not really sure myself, thoug.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Jia He]

Hi Daniel


On 3/14/2018 6:42 AM, Daniel Vacek Wrote:
> On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
> causes a boot hang. This patch fixes the hang making sure the alignment
> never steps back.
>
> Link: http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
> Signed-off-by: Daniel Vacek <neelx@redhat.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: <stable@vger.kernel.org>
> ---
>   mm/page_alloc.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3d974cb2a1a1..e033a6895c6f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>   			 * is not. move_freepages_block() can shift ahead of
>   			 * the valid region but still depends on correct page
>   			 * metadata.
> +			 * Also make sure we never step back.
>   			 */
> -			pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
> +			unsigned long next_pfn;
> +
> +			next_pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>   					~(pageblock_nr_pages-1)) - 1;
> +			if (next_pfn > pfn)
> +				pfn = next_pfn;
It didn't resolve the booting hang issue in my arm64 server.
what if memblock_next_valid_pfn(pfn, end_pfn) is 32 and 
pageblock_nr_pages is 8196?
Thus, next_pfn will be (unsigned long)-1 and be larger than pfn.
So still there is an infinite loop here.

Cheers,
Jia He
>   #endif
>   			continue;
>   		}

-- 
Cheers,
Jia

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Daniel Vacek]

On Thu, Mar 15, 2018 at 12:50 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Thu 15-03-18 02:30:41, Daniel Vacek wrote:
>> On Wed, Mar 14, 2018 at 3:17 PM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Tue 13-03-18 23:42:40, Daniel Vacek wrote:
>> >> On some architectures (reported on arm64) commit 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>> >> causes a boot hang. This patch fixes the hang making sure the alignment
>> >> never steps back.
>> >
>> > I am sorry to be complaining again, but the code is so obscure that I
>>
>> No worries, I'm glad for any review. Which code exactly you do find
>> obscure? This patch or my former fix or the original commit
>> introducing memblock_next_valid_pfn()? Coz I'd agree the original
>> commit looks pretty obscure...
>
> As mentioned in the other email, the whole going back and forth in the
> same loop is just too ugly to live.

It's not really supposed to go back, but I guess you understand.

>> > would _really_ appreciate some more information about what is going
>> > on here. memblock_next_valid_pfn will most likely return a pfn within
>> > the same memblock and the alignment will move it before the old pfn
>> > which is not valid - so the block has some holes. Is that correct?
>>
>> I do not understand what you mean by 'pfn within the same memblock'?
>
> Sorry, I should have said in the same pageblock
>
>> And by 'the block has some holes'?
>
> memblock_next_valid_pfn clearly returns pfn which is within a pageblock
> and that is why we do not initialize pages in the begining of the block
> while move_freepages_block does really expect the full pageblock to be
> initialized properly. That is the fundamental problem, right?

Yes, that's correct.

>> memblock has types 'memory' (as usable memory) and 'reserved' (for
>> unusable mem), if I understand correctly.
>
> We might not have struct pages for invalid pfns. That really depends on
> the memory mode. Sure sparse mem model will usually allocate struct
> pages for whole memory sections but that is not universally true and
> adding such a suble assumption is simply wrong.

This is gray area for me. But if I understand correctly this
assumption comes from the code. It was already there and got broken
hence I was trying to keep it. If anything needs redesigning I'm all
for it. But I was just calming the fire here. I only didn't test on
arm, which seems to be the only one different.

> I suspect you are making strong assumptions based on a very specific
> implementation which might be not true in general. That was the feeling
> I've had since the patch was proposed for the first time. This is such a
> cluttered area that I am not really sure myself, thoug.

I understand. And again this is likely correct. I'll be glad for any
assistance here. My limited knowledge is the primary cause for lack of
relevant details I guess. What I checked looks like pfn_valid is a
generic function used by all arches but arm, which seems to be the
only one to implement CONFIG_HAVE_ARCH_PFN_VALID if I didn't miss
anything. So if this config is enabled on arm, it uses it's own
version of pfn_valid(). If not, I'd expect all arches behave the same.
That's where my assumption comes from.

> --
> Michal Hocko
> SUSE Labs

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Daniel Vacek]

On Thu, Mar 15, 2018 at 3:08 PM, Jia He <hejianet@gmail.com> wrote:
> Hi Daniel
>
>
>
> On 3/14/2018 6:42 AM, Daniel Vacek Wrote:
>>
>> On some architectures (reported on arm64) commit 864b75f9d6b01
>> ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>> causes a boot hang. This patch fixes the hang making sure the alignment
>> never steps back.
>>
>> Link:
>> http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
>> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock
>> alignment")
>> Signed-off-by: Daniel Vacek <neelx@redhat.com>
>> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Paul Burton <paul.burton@imgtec.com>
>> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: <stable@vger.kernel.org>
>> ---
>>   mm/page_alloc.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 3d974cb2a1a1..e033a6895c6f 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size,
>> int nid, unsigned long zone,
>>                          * is not. move_freepages_block() can shift ahead
>> of
>>                          * the valid region but still depends on correct
>> page
>>                          * metadata.
>> +                        * Also make sure we never step back.
>>                          */
>> -                       pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>> +                       unsigned long next_pfn;
>> +
>> +                       next_pfn = (memblock_next_valid_pfn(pfn, end_pfn)
>> &
>>                                         ~(pageblock_nr_pages-1)) - 1;
>> +                       if (next_pfn > pfn)
>> +                               pfn = next_pfn;
>
> It didn't resolve the booting hang issue in my arm64 server.
> what if memblock_next_valid_pfn(pfn, end_pfn) is 32 and pageblock_nr_pages
> is 8196?
> Thus, next_pfn will be (unsigned long)-1 and be larger than pfn.
> So still there is an infinite loop here.

Hi Jia,

Yeah, looks like another uncovered case. Noone reported this so far.
Anyways upstream reverted all this for now and we're discussing the
right approach here.

In any case thanks for this report. Can you share something like below
from your machine?

   Booting Linux on physical CPU 0x0000000000 [0x410fd034]
   Linux version 4.16.0-rc5-00004-gfc6eabbbf8ef-dirty (ard@dogfood) ...
   Machine model: Socionext Developer Box
   earlycon: pl11 at MMIO 0x000000002a400000 (options '')
   bootconsole [pl11] enabled
   efi: Getting EFI parameters from FDT:
   efi: EFI v2.70 by Linaro
   efi:  SMBIOS 3.0=0xff580000  ESRT=0xf9948198  MEMATTR=0xf83b1a98
RNG=0xff7ac898
   random: fast init done
   efi: seeding entropy pool
   esrt: Reserving ESRT space from 0x00000000f9948198 to 0x00000000f99481d0.
   cma: Reserved 16 MiB at 0x00000000fd800000
   NUMA: No NUMA configuration found
   NUMA: Faking a node at [mem 0x0000000000000000-0x0000000fffffffff]
   NUMA: NODE_DATA [mem 0xffffd8d80-0xffffda87f]
   Zone ranges:
     DMA32    [mem 0x0000000080000000-0x00000000ffffffff]
     Normal   [mem 0x0000000100000000-0x0000000fffffffff]
   Movable zone start for each node
   Early memory node ranges
     node   0: [mem 0x0000000080000000-0x00000000febeffff]
     node   0: [mem 0x00000000febf0000-0x00000000fefcffff]
     node   0: [mem 0x00000000fefd0000-0x00000000ff43ffff]
     node   0: [mem 0x00000000ff440000-0x00000000ff7affff]
     node   0: [mem 0x00000000ff7b0000-0x00000000ffffffff]
     node   0: [mem 0x0000000880000000-0x0000000fffffffff]
   Initmem setup node 0 [mem 0x0000000080000000-0x0000000fffffffff]


Thank you.

--nX

> Cheers,
> Jia He
>>
>>   #endif
>>                         continue;
>>                 }
>
>
> --
> Cheers,
> Jia
>

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Jia He]

On 3/15/2018 11:39 PM, Daniel Vacek Wrote:
> On Thu, Mar 15, 2018 at 3:08 PM, Jia He <hejianet@gmail.com> wrote:
>> Hi Daniel
>>
>>
>>
>> On 3/14/2018 6:42 AM, Daniel Vacek Wrote:
>>> On some architectures (reported on arm64) commit 864b75f9d6b01
>>> ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>>> causes a boot hang. This patch fixes the hang making sure the alignment
>>> never steps back.
>>>
>>> Link:
>>> http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
>>> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock
>>> alignment")
>>> Signed-off-by: Daniel Vacek <neelx@redhat.com>
>>> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>>> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Mel Gorman <mgorman@techsingularity.net>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>    mm/page_alloc.c | 7 ++++++-
>>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index 3d974cb2a1a1..e033a6895c6f 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size,
>>> int nid, unsigned long zone,
>>>                           * is not. move_freepages_block() can shift ahead
>>> of
>>>                           * the valid region but still depends on correct
>>> page
>>>                           * metadata.
>>> +                        * Also make sure we never step back.
>>>                           */
>>> -                       pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>>> +                       unsigned long next_pfn;
>>> +
>>> +                       next_pfn = (memblock_next_valid_pfn(pfn, end_pfn)
>>> &
>>>                                          ~(pageblock_nr_pages-1)) - 1;
>>> +                       if (next_pfn > pfn)
>>> +                               pfn = next_pfn;
>> It didn't resolve the booting hang issue in my arm64 server.
>> what if memblock_next_valid_pfn(pfn, end_pfn) is 32 and pageblock_nr_pages
>> is 8196?
>> Thus, next_pfn will be (unsigned long)-1 and be larger than pfn.
>> So still there is an infinite loop here.
> Hi Jia,
>
> Yeah, looks like another uncovered case. Noone reported this so far.
> Anyways upstream reverted all this for now and we're discussing the
> right approach here.
>
> In any case thanks for this report. Can you share something like below
> from your machine?
sure.
[    0.000000] NUMA: Faking a node at [mem 
0x0000000000000000-0x00000017ffffffff]
[    0.000000] NUMA: NODE_DATA [mem 0x17ffffcb80-0x17ffffffff]
[    0.000000] Zone ranges:
[    0.000000]   DMA32    [mem 0x0000000000200000-0x00000000ffffffff]
[    0.000000]   Normal   [mem 0x0000000100000000-0x00000017ffffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000200000-0x000000000021ffff]
[    0.000000]   node   0: [mem 0x0000000000820000-0x000000000307ffff]
[    0.000000]   node   0: [mem 0x0000000003080000-0x000000000308ffff]
[    0.000000]   node   0: [mem 0x0000000003090000-0x00000000031fffff]
[    0.000000]   node   0: [mem 0x0000000003200000-0x00000000033fffff]
[    0.000000]   node   0: [mem 0x0000000003410000-0x000000000563ffff]
[    0.000000]   node   0: [mem 0x0000000005640000-0x000000000567ffff]
[    0.000000]   node   0: [mem 0x0000000005680000-0x00000000056dffff]
[    0.000000]   node   0: [mem 0x00000000056e0000-0x00000000086fffff]
[    0.000000]   node   0: [mem 0x0000000008700000-0x000000000871ffff]
[    0.000000]   node   0: [mem 0x0000000008720000-0x000000000894ffff]
[    0.000000]   node   0: [mem 0x0000000008950000-0x0000000008baffff]
[    0.000000]   node   0: [mem 0x0000000008bb0000-0x0000000008bcffff]
[    0.000000]   node   0: [mem 0x0000000008bd0000-0x0000000008c4ffff]
[    0.000000]   node   0: [mem 0x0000000008c50000-0x0000000008e2ffff]
[    0.000000]   node   0: [mem 0x0000000008e30000-0x0000000008e4ffff]
[    0.000000]   node   0: [mem 0x0000000008e50000-0x0000000008fcffff]
[    0.000000]   node   0: [mem 0x0000000008fd0000-0x000000000910ffff]
[    0.000000]   node   0: [mem 0x0000000009110000-0x00000000092effff]
[    0.000000]   node   0: [mem 0x00000000092f0000-0x000000000930ffff]
[    0.000000]   node   0: [mem 0x0000000009310000-0x000000000963ffff]
[    0.000000]   node   0: [mem 0x0000000009640000-0x000000000e61ffff]
[    0.000000]   node   0: [mem 0x000000000e620000-0x000000000e64ffff]
[    0.000000]   node   0: [mem 0x000000000e650000-0x000000000fffffff]
[    0.000000]   node   0: [mem 0x0000000010800000-0x0000000017feffff]
[    0.000000]   node   0: [mem 0x000000001c000000-0x000000001c00ffff]
[    0.000000]   node   0: [mem 0x000000001c010000-0x000000001c7fffff]
[    0.000000]   node   0: [mem 0x000000001c810000-0x000000007efbffff]
[    0.000000]   node   0: [mem 0x000000007efc0000-0x000000007efdffff]
[    0.000000]   node   0: [mem 0x000000007efe0000-0x000000007efeffff]
[    0.000000]   node   0: [mem 0x000000007eff0000-0x000000007effffff]
[    0.000000]   node   0: [mem 0x000000007f000000-0x00000017ffffffff]
[    0.000000] Initmem setup node 0 [mem 
0x0000000000200000-0x00000017ffffffff]

-- 
Cheers,
Jia

Re: [PATCH] mm/page_alloc: fix boot hang in memmap_init_zone

[Jia He]

On 3/15/2018 11:39 PM, Daniel Vacek Wrote:
> On Thu, Mar 15, 2018 at 3:08 PM, Jia He <hejianet@gmail.com> wrote:
>> Hi Daniel
>>
>>
>>
>> On 3/14/2018 6:42 AM, Daniel Vacek Wrote:
>>> On some architectures (reported on arm64) commit 864b75f9d6b01
>>> ("mm/page_alloc: fix memmap_init_zone pageblock alignment")
>>> causes a boot hang. This patch fixes the hang making sure the alignment
>>> never steps back.
>>>
>>> Link:
>>> http://lkml.kernel.org/r/0485727b2e82da7efbce5f6ba42524b429d0391a.1520011945.git.neelx@redhat.com
>>> Fixes: 864b75f9d6b01 ("mm/page_alloc: fix memmap_init_zone pageblock
>>> alignment")
>>> Signed-off-by: Daniel Vacek <neelx@redhat.com>
>>> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>>> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Mel Gorman <mgorman@techsingularity.net>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: Paul Burton <paul.burton@imgtec.com>
>>> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>    mm/page_alloc.c | 7 ++++++-
>>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index 3d974cb2a1a1..e033a6895c6f 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -5364,9 +5364,14 @@ void __meminit memmap_init_zone(unsigned long size,
>>> int nid, unsigned long zone,
>>>                           * is not. move_freepages_block() can shift ahead
>>> of
>>>                           * the valid region but still depends on correct
>>> page
>>>                           * metadata.
>>> +                        * Also make sure we never step back.
>>>                           */
>>> -                       pfn = (memblock_next_valid_pfn(pfn, end_pfn) &
>>> +                       unsigned long next_pfn;
>>> +
>>> +                       next_pfn = (memblock_next_valid_pfn(pfn, end_pfn)
>>> &
>>>                                          ~(pageblock_nr_pages-1)) - 1;
>>> +                       if (next_pfn > pfn)
>>> +                               pfn = next_pfn;
>> It didn't resolve the booting hang issue in my arm64 server.
>> what if memblock_next_valid_pfn(pfn, end_pfn) is 32 and pageblock_nr_pages
>> is 8196?
>> Thus, next_pfn will be (unsigned long)-1 and be larger than pfn.
>> So still there is an infinite loop here.
> Hi Jia,
>
> Yeah, looks like another uncovered case. Noone reported this so far.
> Anyways upstream reverted all this for now and we're discussing the
> right approach here.
>
> In any case thanks for this report. Can you share something like below
> from your machine?
sure.
[A A A  0.000000] NUMA: Faking a node at [mem 
0x0000000000000000-0x00000017ffffffff]
[A A A  0.000000] NUMA: NODE_DATA [mem 0x17ffffcb80-0x17ffffffff]
[A A A  0.000000] Zone ranges:
[A A A  0.000000]A A  DMA32A A A  [mem 0x0000000000200000-0x00000000ffffffff]
[A A A  0.000000]A A  NormalA A  [mem 0x0000000100000000-0x00000017ffffffff]
[A A A  0.000000] Movable zone start for each node
[A A A  0.000000] Early memory node ranges
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000000200000-0x000000000021ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000000820000-0x000000000307ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000003080000-0x000000000308ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000003090000-0x00000000031fffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000003200000-0x00000000033fffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000003410000-0x000000000563ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000005640000-0x000000000567ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000005680000-0x00000000056dffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x00000000056e0000-0x00000000086fffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008700000-0x000000000871ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008720000-0x000000000894ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008950000-0x0000000008baffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008bb0000-0x0000000008bcffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008bd0000-0x0000000008c4ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008c50000-0x0000000008e2ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008e30000-0x0000000008e4ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008e50000-0x0000000008fcffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000008fd0000-0x000000000910ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000009110000-0x00000000092effff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x00000000092f0000-0x000000000930ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000009310000-0x000000000963ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000009640000-0x000000000e61ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000000e620000-0x000000000e64ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000000e650000-0x000000000fffffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x0000000010800000-0x0000000017feffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000001c000000-0x000000001c00ffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000001c010000-0x000000001c7fffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000001c810000-0x000000007efbffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000007efc0000-0x000000007efdffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000007efe0000-0x000000007efeffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000007eff0000-0x000000007effffff]
[A A A  0.000000]A A  nodeA A  0: [mem 0x000000007f000000-0x00000017ffffffff]
[A A A  0.000000] Initmem setup node 0 [mem 
0x0000000000200000-0x00000017ffffffff]

-- 
Cheers,
Jia