| Message ID | 1371113826-1231-1-git-send-email-b32955@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
? 2013?06?13? 16:57, Huang Shijie ??: > If we want to steal 128K memory in the machine_desc->reserve() hook, we > will hang up immediately. > > The hang reason is like this: > > [1] Stealing 128K makes the left memory is not aligned with the SECTION_SIZE. > > [2] So when the map_lowmem() tries to maps the lowmem memory banks, > it will call the memblock_alloc(in early_alloc_aligned()) to allocate > a page to store the pte. This pte page is in the unaligned region > which is not mapped yet. > > [3] And when we use the memset() in the early_alloc_aligned(), we will hang > right now. > > [4] The hang only occurs in the map_lowmem(). After the map_lowmem(), we have > setup the PTE mappings. So in the later places, such as > dma_contiguous_remap(), the hang will never occurs, > > This patch adds a global variable, in_map_lowmem, to check if we are in > the map_lowmem() or not. If we are in the map_lowmem(), and we steal > a SECTION_SIZE unaligned memory, we will use the memblock_alloc_base() > to allocate the pte page. The @max_addr for memblock_alloc_base() is the > last mapped address. > > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > arch/arm/mm/mmu.c | 34 ++++++++++++++++++++++++++++++---- > 1 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c > index faa36d7..56d1a22 100644 > --- a/arch/arm/mm/mmu.c > +++ b/arch/arm/mm/mmu.c > @@ -113,6 +113,8 @@ static struct cachepolicy cache_policies[] __initdata = { > } > }; > > +static bool in_map_lowmem __initdata; > + > #ifdef CONFIG_CPU_CP15 > /* > * These are useful for identifying cache coherency > @@ -595,10 +597,32 @@ static void __init *early_alloc(unsigned long sz) > return early_alloc_aligned(sz, sz); > } > > -static pte_t * __init early_pte_alloc(pmd_t *pmd, unsigned long addr, unsigned long prot) > +static void __init *early_alloc_max_addr(unsigned long sz, phys_addr_t maddr) > +{ > + void *ptr; > + > + if (maddr == MEMBLOCK_ALLOC_ACCESSIBLE) > + return early_alloc_aligned(sz, sz); > + > + ptr = __va(memblock_alloc_base(sz, sz, maddr)); > + memset(ptr, 0, sz); > + return ptr; > +} > + > +static pte_t * __init early_pte_alloc(pmd_t *pmd, unsigned long addr, > + unsigned long end, unsigned long prot) > { > if (pmd_none(*pmd)) { > - pte_t *pte = early_alloc(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE); > + pte_t *pte; > + phys_addr_t maddr = MEMBLOCK_ALLOC_ACCESSIBLE; > + > + if (in_map_lowmem && (end & SECTION_MASK)) { > + end &= PGDIR_MASK; > + BUG_ON(!end); > + maddr = __virt_to_phys(end); > + } > + pte = early_alloc_max_addr(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE, > + maddr); > __pmd_populate(pmd, __pa(pte), prot); > } > BUG_ON(pmd_bad(*pmd)); > @@ -609,7 +633,7 @@ static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr, > unsigned long end, unsigned long pfn, > const struct mem_type *type) > { > - pte_t *pte = early_pte_alloc(pmd, addr, type->prot_l1); > + pte_t *pte = early_pte_alloc(pmd, addr, end, type->prot_l1); > do { > set_pte_ext(pte, pfn_pte(pfn, __pgprot(type->prot_pte)), 0); > pfn++; > @@ -1253,7 +1277,7 @@ static void __init kmap_init(void) > { > #ifdef CONFIG_HIGHMEM > pkmap_page_table = early_pte_alloc(pmd_off_k(PKMAP_BASE), > - PKMAP_BASE, _PAGE_KERNEL_TABLE); > + PKMAP_BASE, 0, _PAGE_KERNEL_TABLE); > #endif > } > > @@ -1261,6 +1285,7 @@ static void __init map_lowmem(void) > { > struct memblock_region *reg; > > + in_map_lowmem = 1; > /* Map all the lowmem memory banks. */ > for_each_memblock(memory, reg) { > phys_addr_t start = reg->base; > @@ -1279,6 +1304,7 @@ static void __init map_lowmem(void) > > create_mapping(&map); > } > + in_map_lowmem = 0; > } > > /* just a ping.
On Thu, Jun 13, 2013 at 09:57:05AM +0100, Huang Shijie wrote: > If we want to steal 128K memory in the machine_desc->reserve() hook, we > will hang up immediately. > > The hang reason is like this: > > [1] Stealing 128K makes the left memory is not aligned with the SECTION_SIZE. > > [2] So when the map_lowmem() tries to maps the lowmem memory banks, > it will call the memblock_alloc(in early_alloc_aligned()) to allocate > a page to store the pte. This pte page is in the unaligned region > which is not mapped yet. > > [3] And when we use the memset() in the early_alloc_aligned(), we will hang > right now. > > [4] The hang only occurs in the map_lowmem(). After the map_lowmem(), we have > setup the PTE mappings. So in the later places, such as > dma_contiguous_remap(), the hang will never occurs, > > This patch adds a global variable, in_map_lowmem, to check if we are in > the map_lowmem() or not. If we are in the map_lowmem(), and we steal > a SECTION_SIZE unaligned memory, we will use the memblock_alloc_base() > to allocate the pte page. The @max_addr for memblock_alloc_base() is the > last mapped address. Wouldn't this be better achieved with a parameter, rather than a global state variable? That said, I don't completely follow why memblock_alloc is giving you back an unmapped physical address. It sounds like we're freeing too much as part of the stealing (or simply that stealing has to be section aligned), but memblock only deals with physical addresses. Could you elaborate please? Will
On Tue, Jun 18, 2013 at 04:29:05PM +0100, Will Deacon wrote: > Wouldn't this be better achieved with a parameter, rather than a global > state variable? That said, I don't completely follow why memblock_alloc is > giving you back an unmapped physical address. It sounds like we're freeing > too much as part of the stealing (or simply that stealing has to be section > aligned), but memblock only deals with physical addresses. > > Could you elaborate please? It's a catch-22 situation. memblock allocates from the top of usable memory. While setting up the page tables for the second time, we insert section mappings. If the last mapping is not section sized, we will try to set it up using page mappings. For this, we need to allocate L2 page tables from memblock. memblock returns a 4K page in the last non-section sized mapping - which we're trying to setup, and hence is not yet mapped. This is why I've always said - if you steal memory from memblock, it _must_ be aligned to 1MB (the section size) to avoid this. Not only that, but we didn't _used_ to allow page-sized mappings for MT_MEMORY - that got added for OMAP's SRAM support.
? 2013?06?19? 00:52, Russell King - ARM Linux ??: > This is why I've always said - if you steal memory from memblock, it > _must_ be aligned to 1MB (the section size) to avoid this. Not only firstly, it's a little waste to _must_ steal 1M for some board with less memory such as 512M memory; secondly, if we do not support the section unaligned mapping, we should remove the alloc_init_pte() in alloc_init_pmd(), add a comment to tell that we do not support the section unaligned mapping. thanks Huang Shijie
On Wed, Jun 19, 2013 at 10:36:06AM +0800, Huang Shijie wrote: > ? 2013?06?19? 00:52, Russell King - ARM Linux ??: >> This is why I've always said - if you steal memory from memblock, it >> _must_ be aligned to 1MB (the section size) to avoid this. Not only > > firstly, it's a little waste to _must_ steal 1M for some board with less > memory such as 512M memory; You're complaining about 512M memory? Christ. Some of my machines which I run here have as little as 32M of memory! 1M is nothing in 512M. > secondly, if we do not support the section unaligned mapping, we should > remove the alloc_init_pte() in alloc_init_pmd(), > add a comment to tell that we do not support the section unaligned mapping. No, because that breaks a whole load of other non-memory mappings.
? 2013?06?19? 16:28, Russell King - ARM Linux ??: > On Wed, Jun 19, 2013 at 10:36:06AM +0800, Huang Shijie wrote: >> ? 2013?06?19? 00:52, Russell King - ARM Linux ??: >>> This is why I've always said - if you steal memory from memblock, it >>> _must_ be aligned to 1MB (the section size) to avoid this. Not only >> firstly, it's a little waste to _must_ steal 1M for some board with less >> memory such as 512M memory; > You're complaining about 512M memory? Christ. Some of my machines > which I run here have as little as 32M of memory! 1M is nothing in > 512M. > My meaning was : we only need 128K in some case, why we waste other 896K? IMHO, we should treasure the memory. If you think there is no need to fix this issue, I am ok too. >> secondly, if we do not support the section unaligned mapping, we should >> remove the alloc_init_pte() in alloc_init_pmd(), >> add a comment to tell that we do not support the section unaligned mapping. > No, because that breaks a whole load of other non-memory mappings. > yes. you are right. I forgot the other mappings besides the map_lowmem(). thanks Huang Shijie
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c index faa36d7..56d1a22 100644 --- a/arch/arm/mm/mmu.c +++ b/arch/arm/mm/mmu.c @@ -113,6 +113,8 @@ static struct cachepolicy cache_policies[] __initdata = { } }; +static bool in_map_lowmem __initdata; + #ifdef CONFIG_CPU_CP15 /* * These are useful for identifying cache coherency @@ -595,10 +597,32 @@ static void __init *early_alloc(unsigned long sz) return early_alloc_aligned(sz, sz); } -static pte_t * __init early_pte_alloc(pmd_t *pmd, unsigned long addr, unsigned long prot) +static void __init *early_alloc_max_addr(unsigned long sz, phys_addr_t maddr) +{ + void *ptr; + + if (maddr == MEMBLOCK_ALLOC_ACCESSIBLE) + return early_alloc_aligned(sz, sz); + + ptr = __va(memblock_alloc_base(sz, sz, maddr)); + memset(ptr, 0, sz); + return ptr; +} + +static pte_t * __init early_pte_alloc(pmd_t *pmd, unsigned long addr, + unsigned long end, unsigned long prot) { if (pmd_none(*pmd)) { - pte_t *pte = early_alloc(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE); + pte_t *pte; + phys_addr_t maddr = MEMBLOCK_ALLOC_ACCESSIBLE; + + if (in_map_lowmem && (end & SECTION_MASK)) { + end &= PGDIR_MASK; + BUG_ON(!end); + maddr = __virt_to_phys(end); + } + pte = early_alloc_max_addr(PTE_HWTABLE_OFF + PTE_HWTABLE_SIZE, + maddr); __pmd_populate(pmd, __pa(pte), prot); } BUG_ON(pmd_bad(*pmd)); @@ -609,7 +633,7 @@ static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr, unsigned long end, unsigned long pfn, const struct mem_type *type) { - pte_t *pte = early_pte_alloc(pmd, addr, type->prot_l1); + pte_t *pte = early_pte_alloc(pmd, addr, end, type->prot_l1); do { set_pte_ext(pte, pfn_pte(pfn, __pgprot(type->prot_pte)), 0); pfn++; @@ -1253,7 +1277,7 @@ static void __init kmap_init(void) { #ifdef CONFIG_HIGHMEM pkmap_page_table = early_pte_alloc(pmd_off_k(PKMAP_BASE), - PKMAP_BASE, _PAGE_KERNEL_TABLE); + PKMAP_BASE, 0, _PAGE_KERNEL_TABLE); #endif } @@ -1261,6 +1285,7 @@ static void __init map_lowmem(void) { struct memblock_region *reg; + in_map_lowmem = 1; /* Map all the lowmem memory banks. */ for_each_memblock(memory, reg) { phys_addr_t start = reg->base; @@ -1279,6 +1304,7 @@ static void __init map_lowmem(void) create_mapping(&map); } + in_map_lowmem = 0; } /*
If we want to steal 128K memory in the machine_desc->reserve() hook, we will hang up immediately. The hang reason is like this: [1] Stealing 128K makes the left memory is not aligned with the SECTION_SIZE. [2] So when the map_lowmem() tries to maps the lowmem memory banks, it will call the memblock_alloc(in early_alloc_aligned()) to allocate a page to store the pte. This pte page is in the unaligned region which is not mapped yet. [3] And when we use the memset() in the early_alloc_aligned(), we will hang right now. [4] The hang only occurs in the map_lowmem(). After the map_lowmem(), we have setup the PTE mappings. So in the later places, such as dma_contiguous_remap(), the hang will never occurs, This patch adds a global variable, in_map_lowmem, to check if we are in the map_lowmem() or not. If we are in the map_lowmem(), and we steal a SECTION_SIZE unaligned memory, we will use the memblock_alloc_base() to allocate the pte page. The @max_addr for memblock_alloc_base() is the last mapped address. Signed-off-by: Huang Shijie <b32955@freescale.com> --- arch/arm/mm/mmu.c | 34 ++++++++++++++++++++++++++++++---- 1 files changed, 30 insertions(+), 4 deletions(-)