All of lore.kernel.org
 help / color / mirror / Atom feed
From: grygorii.strashko@ti.com (Grygorii Strashko)
To: linux-arm-kernel@lists.infradead.org
Subject: ARM: issue with memory reservation from DT
Date: Thu, 16 Oct 2014 20:32:19 +0300	[thread overview]
Message-ID: <54400123.7040806@ti.com> (raw)
In-Reply-To: <20141015175025.GJ27405@n2100.arm.linux.org.uk>

Hi Russell,
On 10/15/2014 08:50 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 15, 2014 at 08:18:18PM +0300, Grygorii Strashko wrote:
>> 3) If I apply below change - I can boot:
>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>> index c031063..85ad92b 100644
>> --- a/arch/arm/kernel/setup.c
>> +++ b/arch/arm/kernel/setup.c
>> @@ -917,8 +917,8 @@ void __init setup_arch(char **cmdline_p)
>>   
>>          early_paging_init(mdesc, lookup_processor_type(read_cpuid_id()));
>>          setup_dma_zone(mdesc);
>> -       sanity_check_meminfo();
>>          arm_memblock_init(mdesc);
>> +       sanity_check_meminfo();
>>   
>>          paging_init(mdesc);
>>          request_standard_resources(mdesc);
>>
>> ^^ not sure if it totally safe, because dma_contiguous_reserve(arm_dma_limit);
>> is called from inside arm_memblock_init() and it does bootmem allocations.
> 
> It isn't.  sanity_check_meminfo() _must_ be called before arm_memblock_init()
> so that sanity_check_meminfo() can adjust the passed memory description to
> remove stuff which is inappropriate for the configuration, before it is
> passed to memblock.
> 
>> Sort Summary:
>> It looks like all static memory reservation and memory stealing's
>> (calling of memblock_remove()) have to be done before any other
>> operations and before calculating ARM memory limits.
> 
> No, that should not be the case.  The way it is /supposed/ to work is:
> 
> - We obtain the memory information and pass it into memblock
> - We sanity check the memory in memblock, removing memory which we
>    deem to be unacceptable for the kernel configuration via
>    memblock_remove().  Also calculate the highest address we are
>    prepared to allocate, which is set to the top of the first chunk
>    of memory, or the top of lowmem.
> - We then see about reserving memory from memblock.  This marks memory
>    as reserved, or in certain cases where we actually want to prevent
>    the kernel taking control of the memory, we completely remove the
>    memory from memblock (via memblock_remove).

In my case amount of removed memory is so high that there is no room
for Highmem anymore.

memblock.memory.regions[0].base + size < arm_lowmem_limit
and arm_lowmem_limit == memblock.current_limit

> 
> Memory removed via memblock_remove() is then not available for any
> allocations, and should not be touched by the kernel in any way from
> that point on.
> 
> It doesn't matter that the memblock limit is still set higher, because
> the memory has been removed from the available memory pool, it should
> not be allocated.
> 

You are right in general, but seems problem is not in memblock itself :(
The problem is with  memory control variables like:
 - arm_lowmem_limit
 - max_low_pfn
 - max_pfn

The last thing I've found that issue happens when in 
bootmem_init()->find_limits() the max_low variable got value greater than
max_high: max_low_pfn > max_pfn.

Then kernel crashes somewhere inside free_all_bootmem();

Below hack allows to boot:
+++ b/arch/arm/mm/init.c
@@ -140,6 +140,8 @@ static void __init find_limits(unsigned long *min, unsigned long *max_low,
        *max_low = PFN_DOWN(memblock_get_current_limit());
        *min = PFN_UP(memblock_start_of_DRAM());
        *max_high = PFN_DOWN(memblock_end_of_DRAM());
+       if (*max_low > *max_high)
+               *max_low = *max_high;
 }

Regards,
-grygorii

  reply	other threads:[~2014-10-16 17:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15 17:18 ARM: issue with memory reservation from DT Grygorii Strashko
2014-10-15 17:50 ` Russell King - ARM Linux
2014-10-16 17:32   ` Grygorii Strashko [this message]
2014-10-17  9:10     ` Laura Abbott
2014-10-17 10:21       ` Grygorii Strashko
2014-10-17 16:54         ` Laura Abbott
2014-10-20 20:48           ` Laura Abbott
2014-10-21 17:01             ` Grygorii Strashko
2014-10-21 18:32               ` Laura Abbott
2014-10-17 11:36     ` Santosh Shilimkar
2014-10-21 17:02       ` Grygorii Strashko
2014-10-21 17:16         ` Russell King - ARM Linux
2014-10-24 18:16           ` Laura Abbott

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54400123.7040806@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.