[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [microblaze-uclinux] [patch] boot memory allocator cleanup 3(take 2)



Hi Yashi and others,

I'm back looking at the boot memory allocation stuff - basically trying 
to ARM-ify the memory allocation as a step towards supporting 
discontigmem and so on..

Anyway I think I've found a bug in the current setup after the last 
series of mm patches.  see below for explanation:

Yasushi SHOJI wrote:

> At Tue, 13 Apr 2004 21:04:25 +1000,
> John Williams wrote:
 >
>>Yasushi SHOJI wrote:
>>
>>>>Is there anything that assumes .bss will be page aligned?  Otherwise, 
>>>>it's some kind of off-by-one or align/reounding error, I think.
>>>
>>>
>>>there are some code that assuming boundary is page aligned. I'll look
>>>into it.
>>
>>The easy fix is just to put
>>
>>.bss ALIGN(4096) {
>>    ...
>>}
>>
>>in the link script... but it smells like a hack.. ;-)
> 
> 
> It seems I found the problem.  in the old days, we PAGE_ALIGN'ed
> __init_begin and __init_end.  This won't work once we changed .bss not
> to aligned to page, because that means we are freeing beginig of
> section.
> 
> this is the relative part of the fix in the attached patch.

Looking at it again, there is very good reason to page align the 
__init_begin and __init_end.  In the free_init_mem() routine, it gets 
the __init_start and __init_end addresses, converts them to pages, and 
frees the pages.

But, if these *aren't* page aligned, then we get exactly the same 
problem as before, because we free a page that is only partially .init 
section, and partially whatever comes next (.bss - bad news!!).

So, we must surely page align  __init_start and __init_end.  I think we 
removed that as an efficiency thing, but it makes no difference except 
to shrink the kernel binary image slightly.  At runtime it wastes no 
memory, because the pages are freed anyway. Worst case image size 
wastage is one page, or 4K....

The fix would simply be in the link script, at the end of the .init section:

  . = ALIGN(4096);
  __init_end = .;
}

What do you think?

John
___________________________
microblaze-uclinux mailing list
microblaze-uclinux@itee.uq.edu.au
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/