From: Michael Sundius <[email protected]>
Fix 3 problems in the MIPS SPARSEMEM implementation:
1) mem_init() sets/clears PG_reserved on all pages in the HIGHMEM range
without checking to see whether the page descriptor actually exists.
2) bootmem_init() never calls memory_present() on HIGHMEM pages, so
page descriptors are never created for them if SPARSEMEM is enabled.
3) bootmem_init() calls memory_present() on lowmem pages before bootmem
is fully set up. This is bad because memory_present() can allocate
bootmem in some circumstances (e.g. if SPARSEMEM_EXTREME ever got
enabled).
Signed-off-by: Michael Sundius <[email protected]>
Signed-off-by: Kevin Cernekee <[email protected]>
Cc: [email protected]
---
arch/mips/kernel/setup.c | 18 +++++++++++++++++-
arch/mips/mm/init.c | 3 +++
2 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 8ad1d56..1f9f902 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -390,7 +390,6 @@ static void __init bootmem_init(void)
/* Register lowmem ranges */
free_bootmem(PFN_PHYS(start), size << PAGE_SHIFT);
- memory_present(0, start, end);
}
/*
@@ -402,6 +401,23 @@ static void __init bootmem_init(void)
* Reserve initrd memory if needed.
*/
finalize_initrd();
+
+ /*
+ * Call memory_present() on all valid ranges, for SPARSEMEM.
+ * This must be done after setting up bootmem, since memory_present()
+ * may allocate bootmem.
+ */
+ for (i = 0; i < boot_mem_map.nr_map; i++) {
+ unsigned long start, end;
+
+ if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
+ continue;
+
+ start = PFN_UP(boot_mem_map.map[i].addr);
+ end = PFN_DOWN(boot_mem_map.map[i].addr
+ + boot_mem_map.map[i].size);
+ memory_present(0, start, end);
+ }
}
#endif /* CONFIG_SGI_IP27 */
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 279599e..78a4cf2 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -392,6 +392,9 @@ void __init mem_init(void)
for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
struct page *page = pfn_to_page(tmp);
+ if (!pfn_valid(tmp))
+ continue;
+
if (!page_is_ram(tmp)) {
SetPageReserved(page);
continue;
--
1.7.4.2
On 03/31/2011 05:27 PM, Kevin Cernekee wrote:
> From: Michael Sundius<[email protected]>
>
> Fix 3 problems in the MIPS SPARSEMEM implementation:
>
> 1) mem_init() sets/clears PG_reserved on all pages in the HIGHMEM range
> without checking to see whether the page descriptor actually exists.
>
> 2) bootmem_init() never calls memory_present() on HIGHMEM pages, so
> page descriptors are never created for them if SPARSEMEM is enabled.
>
> 3) bootmem_init() calls memory_present() on lowmem pages before bootmem
> is fully set up. This is bad because memory_present() can allocate
> bootmem in some circumstances (e.g. if SPARSEMEM_EXTREME ever got
> enabled).
>
I think this may do the same thing as my patch:
http://patchwork.linux-mips.org/patch/1988/
Although my patch had different motivations, and changes some other
things around too.
David Daney
> Signed-off-by: Michael Sundius<[email protected]>
> Signed-off-by: Kevin Cernekee<[email protected]>
> Cc: [email protected]
> ---
> arch/mips/kernel/setup.c | 18 +++++++++++++++++-
> arch/mips/mm/init.c | 3 +++
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 8ad1d56..1f9f902 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -390,7 +390,6 @@ static void __init bootmem_init(void)
>
> /* Register lowmem ranges */
> free_bootmem(PFN_PHYS(start), size<< PAGE_SHIFT);
> - memory_present(0, start, end);
> }
>
> /*
> @@ -402,6 +401,23 @@ static void __init bootmem_init(void)
> * Reserve initrd memory if needed.
> */
> finalize_initrd();
> +
> + /*
> + * Call memory_present() on all valid ranges, for SPARSEMEM.
> + * This must be done after setting up bootmem, since memory_present()
> + * may allocate bootmem.
> + */
> + for (i = 0; i< boot_mem_map.nr_map; i++) {
> + unsigned long start, end;
> +
> + if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> + continue;
> +
> + start = PFN_UP(boot_mem_map.map[i].addr);
> + end = PFN_DOWN(boot_mem_map.map[i].addr
> + + boot_mem_map.map[i].size);
> + memory_present(0, start, end);
> + }
> }
>
> #endif /* CONFIG_SGI_IP27 */
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 279599e..78a4cf2 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -392,6 +392,9 @@ void __init mem_init(void)
> for (tmp = highstart_pfn; tmp< highend_pfn; tmp++) {
> struct page *page = pfn_to_page(tmp);
>
> + if (!pfn_valid(tmp))
> + continue;
> +
> if (!page_is_ram(tmp)) {
> SetPageReserved(page);
> continue;
On Fri, Apr 1, 2011 at 9:56 AM, David Daney <[email protected]> wrote:
> I think this may do the same thing as my patch:
>
> http://patchwork.linux-mips.org/patch/1988/
>
> Although my patch had different motivations, and changes some other things
> around too.
I noticed that some of the other architectures have started using the
<linux/memblock.h> APIs for memory setup. Do you think this would be
useful on MIPS, as part of a larger refactoring of bootmem_init() ?
What I liked about Michael's fix was that it is simple and
straightforward enough to meet the stable tree criteria. Long term it
would probably be a good idea to clean up the memory init code and get
rid of the cut&paste "for" loops.
David Daney wrote:
>
>
> I think this may do the same thing as my patch:
>
> http://patchwork.linux-mips.org/patch/1988/
>
> Although my patch had different motivations, and changes some other
> things around too.
>
> David Daney
>
I'm not really sure why your kernel or initrd would be in memory was not
within
the range that had been accounted for. are you saying its in high mem?
On 04/01/2011 11:41 AM, Michael Sundius wrote:
> David Daney wrote:
>>
>>
>> I think this may do the same thing as my patch:
>>
>> http://patchwork.linux-mips.org/patch/1988/
>>
>> Although my patch had different motivations, and changes some other
>> things around too.
>>
>> David Daney
>>
> I'm not really sure why your kernel or initrd would be in memory was not
> within
> the range that had been accounted for. are you saying its in high mem?
>
Well the memory initialization code has a bunch of weird rules built in
that prevent some memory from being used.
For example if the kernel resides in a different SPARSE page than the
rest of memory bad things happen because memory_present() was not called
on something that is later freed (when init memory is released).
If I try to put an initrd at a high physical address, the memory below
that is not usable.
My three patches try to make some sense out of the whole thing.
David Daney