[PATCH] do not count pages in holes with sparsemem

[PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

With SPARSEMEM, the single node can contains some holes so there might
be many invalid pages.  For example, with two 256M memory and one 256M
hole, some variables (num_physpage, totalpages, nr_kernel_pages,
nr_all_pages, etc.) will indicate that there are 768MB on this system.
This is not desired because, for example, alloc_large_system_hash()
allocates too many entries.

Use free_area_init_node() with counted zholes_size[] instead of
free_area_init().

For num_physpages, use number of ram pages instead of max_low_pfn.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 802bdd3..d41dee5 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -139,10 +139,36 @@ #endif /* CONFIG_HIGHMEM */
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 extern void pagetable_init(void);
 
+static int __init page_is_ram(unsigned long pagenr)
+{
+	int i;
+
+	for (i = 0; i < boot_mem_map.nr_map; i++) {
+		unsigned long addr, end;
+
+		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
+			/* not usable memory */
+			continue;
+
+		addr = PFN_UP(boot_mem_map.map[i].addr);
+		end = PFN_DOWN(boot_mem_map.map[i].addr +
+			       boot_mem_map.map[i].size);
+
+		if (pagenr >= addr && pagenr < end)
+			return 1;
+	}
+
+	return 0;
+}
+
 void __init paging_init(void)
 {
-	unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
+	unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 	unsigned long max_dma, high, low;
+#ifdef CONFIG_SPARSEMEM
+	unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
+	unsigned long i, j, pfn;
+#endif
 
 	pagetable_init();
 
@@ -174,29 +200,17 @@ #ifdef CONFIG_HIGHMEM
 		zones_size[ZONE_HIGHMEM] = high - low;
 #endif
 
+#ifdef CONFIG_SPARSEMEM
+	pfn = 0;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		for (j = 0; j < zones_size[i]; j++, pfn++)
+			if (!page_is_ram(pfn))
+				zholes_size[i]++;
+	free_area_init_node(0, NODE_DATA(0), zones_size,
+			    __pa(PAGE_OFFSET), zholes_size);
+#else
 	free_area_init(zones_size);
-}
-
-static inline int page_is_ram(unsigned long pagenr)
-{
-	int i;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long addr, end;
-
-		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
-			/* not usable memory */
-			continue;
-
-		addr = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr +
-			       boot_mem_map.map[i].size);
-
-		if (pagenr >= addr && pagenr < end)
-			return 1;
-	}
-
-	return 0;
+#endif
 }
 
 static struct kcore_list kcore_mem, kcore_vmalloc;
@@ -213,9 +227,9 @@ #ifdef CONFIG_HIGHMEM
 #ifdef CONFIG_DISCONTIGMEM
 #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"
 #endif
-	max_mapnr = num_physpages = highend_pfn;
+	max_mapnr = highend_pfn;
 #else
-	max_mapnr = num_physpages = max_low_pfn;
+	max_mapnr = max_low_pfn;
 #endif
 	high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT);
 
@@ -229,6 +243,7 @@ #endif
 			if (PageReserved(pfn_to_page(tmp)))
 				reservedpages++;
 		}
+	num_physpages = ram;
 
 #ifdef CONFIG_HIGHMEM
 	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
@@ -247,6 +262,7 @@ #endif
 		totalhigh_pages++;
 	}
 	totalram_pages += totalhigh_pages;
+	num_physpages += totalhigh_pages;
 #endif
 
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Atsushi Nemoto wrote:
> With SPARSEMEM, the single node can contains some holes so there might
> be many invalid pages.  For example, with two 256M memory and one 256M

Does SPARSEMEM is the only memory model where we can have memory holes ?

> hole, some variables (num_physpage, totalpages, nr_kernel_pages,
> nr_all_pages, etc.) will indicate that there are 768MB on this system.
> This is not desired because, for example, alloc_large_system_hash()
> allocates too many entries.
> 
> Use free_area_init_node() with counted zholes_size[] instead of
> free_area_init().
> 
> For num_physpages, use number of ram pages instead of max_low_pfn.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 802bdd3..d41dee5 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c

[snip]

>  
> @@ -174,29 +200,17 @@ #ifdef CONFIG_HIGHMEM
>  		zones_size[ZONE_HIGHMEM] = high - low;
>  #endif
>  
> +#ifdef CONFIG_SPARSEMEM
> +	pfn = 0;
> +	for (i = 0; i < MAX_NR_ZONES; i++)
> +		for (j = 0; j < zones_size[i]; j++, pfn++)
> +			if (!page_is_ram(pfn))
> +				zholes_size[i]++;
> +	free_area_init_node(0, NODE_DATA(0), zones_size,
> +			    __pa(PAGE_OFFSET), zholes_size);


Does this code really need the ifdef CONFIG_SPARSEMEM ? Can't we make
it generic instead. Only zholes_size[] initialisation really depends
on the memory model. Of course FLATMEM will let zholes_size as is...

If I remember correctly free_area_init_node() takes a pfn number as
fourth parameter: __pa(PAGE_OFFSET) results in a physical address...

BTW why using __pa(OFFSET) ? isn't it going to yield always into 0 ?
At least on MIPS, it's defined as

#define __pa(x)	((unsigned long) (x) - PAGE_OFFSET)

why not using ARCH_PFN_OFFSET instead ?

thanks

		Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Wed, 05 Jul 2006 15:58:52 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> Does SPARSEMEM is the only memory model where we can have memory holes ?
...
> Does this code really need the ifdef CONFIG_SPARSEMEM ? Can't we make
> it generic instead. Only zholes_size[] initialisation really depends
> on the memory model. Of course FLATMEM will let zholes_size as is...
 
Indeed.  Others can have holes, but might be a bit ineffective.

> If I remember correctly free_area_init_node() takes a pfn number as
> fourth parameter: __pa(PAGE_OFFSET) results in a physical address...

You are right.

> BTW why using __pa(OFFSET) ? isn't it going to yield always into 0 ?
> At least on MIPS, it's defined as
> 
> #define __pa(x)	((unsigned long) (x) - PAGE_OFFSET)
> 
> why not using ARCH_PFN_OFFSET instead ?

Indeed.  I copied the code from free_area_init().  I think 0 is enough
for MIPS.  Patch revised.  Thank you for comments.


With some memory model other than FLATMEM, the single node can
contains some holes so there might be many invalid pages.  For
example, with two 256M memory and one 256M hole, some variables
(num_physpage, totalpages, nr_kernel_pages, nr_all_pages, etc.) will
indicate that there are 768MB on this system.  This is not desired
because, for example, alloc_large_system_hash() allocates too many
entries.

Use free_area_init_node() with counted zholes_size[] instead of
free_area_init().

For num_physpages, use number of ram pages instead of max_low_pfn.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 802bdd3..6c68b2a 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -139,10 +139,36 @@ #endif /* CONFIG_HIGHMEM */
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 extern void pagetable_init(void);
 
+static int __init page_is_ram(unsigned long pagenr)
+{
+	int i;
+
+	for (i = 0; i < boot_mem_map.nr_map; i++) {
+		unsigned long addr, end;
+
+		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
+			/* not usable memory */
+			continue;
+
+		addr = PFN_UP(boot_mem_map.map[i].addr);
+		end = PFN_DOWN(boot_mem_map.map[i].addr +
+			       boot_mem_map.map[i].size);
+
+		if (pagenr >= addr && pagenr < end)
+			return 1;
+	}
+
+	return 0;
+}
+
 void __init paging_init(void)
 {
-	unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
+	unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 	unsigned long max_dma, high, low;
+	unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
+#ifndef CONFIG_FLATMEM
+	unsigned long i, j, pfn;
+#endif
 
 	pagetable_init();
 
@@ -174,29 +200,14 @@ #ifdef CONFIG_HIGHMEM
 		zones_size[ZONE_HIGHMEM] = high - low;
 #endif
 
-	free_area_init(zones_size);
-}
-
-static inline int page_is_ram(unsigned long pagenr)
-{
-	int i;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long addr, end;
-
-		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
-			/* not usable memory */
-			continue;
-
-		addr = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr +
-			       boot_mem_map.map[i].size);
-
-		if (pagenr >= addr && pagenr < end)
-			return 1;
-	}
-
-	return 0;
+#ifndef CONFIG_FLATMEM
+	pfn = 0;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		for (j = 0; j < zones_size[i]; j++, pfn++)
+			if (!page_is_ram(pfn))
+				zholes_size[i]++;
+#endif
+	free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
 }
 
 static struct kcore_list kcore_mem, kcore_vmalloc;
@@ -213,9 +224,9 @@ #ifdef CONFIG_HIGHMEM
 #ifdef CONFIG_DISCONTIGMEM
 #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"
 #endif
-	max_mapnr = num_physpages = highend_pfn;
+	max_mapnr = highend_pfn;
 #else
-	max_mapnr = num_physpages = max_low_pfn;
+	max_mapnr = max_low_pfn;
 #endif
 	high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT);
 
@@ -229,6 +240,7 @@ #endif
 			if (PageReserved(pfn_to_page(tmp)))
 				reservedpages++;
 		}
+	num_physpages = ram;
 
 #ifdef CONFIG_HIGHMEM
 	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
@@ -247,6 +259,7 @@ #endif
 		totalhigh_pages++;
 	}
 	totalram_pages += totalhigh_pages;
+	num_physpages += totalhigh_pages;
 #endif
 
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Atsushi Nemoto wrote:
> On Wed, 05 Jul 2006 15:58:52 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>> BTW why using __pa(OFFSET) ? isn't it going to yield always into 0 ?
>> At least on MIPS, it's defined as
>>
>> #define __pa(x)	((unsigned long) (x) - PAGE_OFFSET)
>>
>> why not using ARCH_PFN_OFFSET instead ?
> 
> Indeed.  I copied the code from free_area_init().  I think 0 is enough
> for MIPS.  Patch revised.  Thank you for comments.
> 
> 

Ok thinking more about it, some platforms may have physical memory
that doesn't start at 0. MIPS doesn't support such platform though it
should be fairly easy. In that case __pa should be defined as:

	#define __pa(x)	((unsigned long) (x) - PAGE_OFFSET + PFN_PHYS(ARCH_PFN_OFFSET))

and use in your patch:

	free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, zholes_size);

So I would recommend to use ARCH_PFN_OFFSET.

		Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Thu, 06 Jul 2006 15:12:11 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> Ok thinking more about it, some platforms may have physical memory
> that doesn't start at 0. MIPS doesn't support such platform though it
> should be fairly easy. In that case __pa should be defined as:
> 
> 	#define __pa(x)	((unsigned long) (x) - PAGE_OFFSET + PFN_PHYS(ARCH_PFN_OFFSET))
> 
> and use in your patch:
> 
> 	free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, zholes_size);
> 
> So I would recommend to use ARCH_PFN_OFFSET.

Well, currently ARCH_PFN_OFFSET is defined in
asm-generic/memory_model.h only for FLATMEM case.  I think other
memory models do not need it because it is just a case that a first
hole begins at pfn 0.

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Atsushi Nemoto wrote:
> On Thu, 06 Jul 2006 15:12:11 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>> Ok thinking more about it, some platforms may have physical memory
>> that doesn't start at 0. MIPS doesn't support such platform though it
>> should be fairly easy. In that case __pa should be defined as:
>>
>> 	#define __pa(x)	((unsigned long) (x) - PAGE_OFFSET + PFN_PHYS(ARCH_PFN_OFFSET))
>>
>> and use in your patch:
>>
>> 	free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, zholes_size);
>>
>> So I would recommend to use ARCH_PFN_OFFSET.
> 
> Well, currently ARCH_PFN_OFFSET is defined in
> asm-generic/memory_model.h only for FLATMEM case.  I think other
> memory models do not need it because it is just a case that a first
> hole begins at pfn 0.
> 

That's true, I thought it was defined whatever the mem models...

what about this, on top of your patch ?

-- >8 --

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index c6e684d..eb1b3fc 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -166,8 +166,8 @@ void __init paging_init(void)
 {
 	unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 	unsigned long max_dma, high, low;
-	unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 #ifndef CONFIG_FLATMEM
+	unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 	unsigned long i, j, pfn;
 #endif
 
@@ -207,8 +207,10 @@ #ifndef CONFIG_FLATMEM
 		for (j = 0; j < zones_size[i]; j++, pfn++)
 			if (!page_is_ram(pfn))
 				zholes_size[i]++;
-#endif
 	free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
+#else
+	free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, NULL);
+#endif
 }
 
 static struct kcore_list kcore_mem, kcore_vmalloc;

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

2006/7/6, Franck Bui-Huu <vagabon.xyz@gmail.com>:
> @@ -207,8 +207,10 @@ #ifndef CONFIG_FLATMEM
>                 for (j = 0; j < zones_size[i]; j++, pfn++)
>                         if (!page_is_ram(pfn))
>                                 zholes_size[i]++;
> -#endif
>         free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
> +#else
> +       free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, NULL);

which is equivalent to:

       free_area_init(zones_size);

-- 
               Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Thu, 6 Jul 2006 17:05:35 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> >         free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
> > +#else
> > +       free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, NULL);
> 
> which is equivalent to:
> 
>        free_area_init(zones_size);

Sure.  Then this can be a final proposal?


With some memory model other than FLATMEM, the single node can
contains some holes so there might be many invalid pages.  For
example, with two 256M memory and one 256M hole, some variables
(num_physpage, totalpages, nr_kernel_pages, nr_all_pages, etc.) will
indicate that there are 768MB on this system.  This is not desired
because, for example, alloc_large_system_hash() allocates too many
entries.

Use free_area_init_node() with counted zholes_size[] instead of
free_area_init().

For num_physpages, use number of ram pages instead of max_low_pfn.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 802bdd3..c52497b 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -139,10 +139,36 @@ #endif /* CONFIG_HIGHMEM */
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 extern void pagetable_init(void);
 
+static int __init page_is_ram(unsigned long pagenr)
+{
+	int i;
+
+	for (i = 0; i < boot_mem_map.nr_map; i++) {
+		unsigned long addr, end;
+
+		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
+			/* not usable memory */
+			continue;
+
+		addr = PFN_UP(boot_mem_map.map[i].addr);
+		end = PFN_DOWN(boot_mem_map.map[i].addr +
+			       boot_mem_map.map[i].size);
+
+		if (pagenr >= addr && pagenr < end)
+			return 1;
+	}
+
+	return 0;
+}
+
 void __init paging_init(void)
 {
-	unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
+	unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
 	unsigned long max_dma, high, low;
+#ifndef CONFIG_FLATMEM
+	unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
+	unsigned long i, j, pfn;
+#endif
 
 	pagetable_init();
 
@@ -174,29 +200,16 @@ #ifdef CONFIG_HIGHMEM
 		zones_size[ZONE_HIGHMEM] = high - low;
 #endif
 
+#ifdef CONFIG_FLATMEM
 	free_area_init(zones_size);
-}
-
-static inline int page_is_ram(unsigned long pagenr)
-{
-	int i;
-
-	for (i = 0; i < boot_mem_map.nr_map; i++) {
-		unsigned long addr, end;
-
-		if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
-			/* not usable memory */
-			continue;
-
-		addr = PFN_UP(boot_mem_map.map[i].addr);
-		end = PFN_DOWN(boot_mem_map.map[i].addr +
-			       boot_mem_map.map[i].size);
-
-		if (pagenr >= addr && pagenr < end)
-			return 1;
-	}
-
-	return 0;
+#else
+	pfn = 0;
+	for (i = 0; i < MAX_NR_ZONES; i++)
+		for (j = 0; j < zones_size[i]; j++, pfn++)
+			if (!page_is_ram(pfn))
+				zholes_size[i]++;
+	free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
+#endif
 }
 
 static struct kcore_list kcore_mem, kcore_vmalloc;
@@ -213,9 +226,9 @@ #ifdef CONFIG_HIGHMEM
 #ifdef CONFIG_DISCONTIGMEM
 #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"
 #endif
-	max_mapnr = num_physpages = highend_pfn;
+	max_mapnr = highend_pfn;
 #else
-	max_mapnr = num_physpages = max_low_pfn;
+	max_mapnr = max_low_pfn;
 #endif
 	high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT);
 
@@ -229,6 +242,7 @@ #endif
 			if (PageReserved(pfn_to_page(tmp)))
 				reservedpages++;
 		}
+	num_physpages = ram;
 
 #ifdef CONFIG_HIGHMEM
 	for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
@@ -247,6 +261,7 @@ #endif
 		totalhigh_pages++;
 	}
 	totalram_pages += totalhigh_pages;
+	num_physpages += totalhigh_pages;
 #endif
 
 	codesize =  (unsigned long) &_etext - (unsigned long) &_text;

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

2006/7/6, Atsushi Nemoto <anemo@mba.ocn.ne.jp>:
> On Thu, 6 Jul 2006 17:05:35 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> > >         free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
> > > +#else
> > > +       free_area_init_node(0, NODE_DATA(0), zones_size, ARCH_PFN_OFFSET, NULL);
> >
> > which is equivalent to:
> >
> >        free_area_init(zones_size);
>
> Sure.  Then this can be a final proposal?
>

Sorry for the late answer.

Did you check that show_mem() still works ? I'm not sure about that point.

Thanks
-- 
               Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Sat, 8 Jul 2006 16:39:44 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> Did you check that show_mem() still works ? I'm not sure about that point.

It should work, but this patch would make the output a bit better.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/arch/mips/mm/pgtable.c b/arch/mips/mm/pgtable.c
index 792c6eb..c93aa6c 100644
--- a/arch/mips/mm/pgtable.c
+++ b/arch/mips/mm/pgtable.c
@@ -15,6 +15,8 @@ #ifndef CONFIG_NEED_MULTIPLE_NODES  /* X
 	printk("Free swap:       %6ldkB\n", nr_swap_pages<<(PAGE_SHIFT-10));
 	pfn = max_mapnr;
 	while (pfn-- > 0) {
+		if (!pfn_valid(pfn))
+			continue;
 		page = pfn_to_page(pfn);
 		total++;
 		if (PageHighMem(page))

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

2006/7/8, Atsushi Nemoto <anemo@mba.ocn.ne.jp>:
> On Sat, 8 Jul 2006 16:39:44 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> > Did you check that show_mem() still works ? I'm not sure about that point.
>
> It should work, but this patch would make the output a bit better.
>

well I would say without this patch it should break.

'pfn' takes values between 0 and max_mapnr. This range includes memory
holes, doens't it ? In that case what does
pfn_to_page(pfn_inside_a_hole) ?

-- 
               Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Sat, 8 Jul 2006 18:15:52 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> well I would say without this patch it should break.
> 
> 'pfn' takes values between 0 and max_mapnr. This range includes memory
> holes, doens't it ? In that case what does
> pfn_to_page(pfn_inside_a_hole) ?

Oh, you are right.  The show_mem() must be fixed indeed.

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

2006/7/6, Atsushi Nemoto <anemo@mba.ocn.ne.jp>:
>
> Sure.  Then this can be a final proposal?
>

could you put the diffstat next time ?

>
> With some memory model other than FLATMEM, the single node can
> contains some holes so there might be many invalid pages.  For
> example, with two 256M memory and one 256M hole, some variables
> (num_physpage, totalpages, nr_kernel_pages, nr_all_pages, etc.) will
> indicate that there are 768MB on this system.  This is not desired
> because, for example, alloc_large_system_hash() allocates too many
> entries.
>
> Use free_area_init_node() with counted zholes_size[] instead of
> free_area_init().
>
> For num_physpages, use number of ram pages instead of max_low_pfn.
>
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 802bdd3..c52497b 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -139,10 +139,36 @@ #endif /* CONFIG_HIGHMEM */
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
>  extern void pagetable_init(void);
>
> +static int __init page_is_ram(unsigned long pagenr)
> +{
> +       int i;
> +
> +       for (i = 0; i < boot_mem_map.nr_map; i++) {
> +               unsigned long addr, end;
> +
> +               if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> +                       /* not usable memory */
> +                       continue;
> +
> +               addr = PFN_UP(boot_mem_map.map[i].addr);
> +               end = PFN_DOWN(boot_mem_map.map[i].addr +
> +                              boot_mem_map.map[i].size);
> +
> +               if (pagenr >= addr && pagenr < end)
> +                       return 1;
> +       }
> +
> +       return 0;
> +}
> +
>  void __init paging_init(void)
>  {
> -       unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
> +       unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
>         unsigned long max_dma, high, low;
> +#ifndef CONFIG_FLATMEM
> +       unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
> +       unsigned long i, j, pfn;
> +#endif
>
>         pagetable_init();
>
> @@ -174,29 +200,16 @@ #ifdef CONFIG_HIGHMEM
>                 zones_size[ZONE_HIGHMEM] = high - low;
>  #endif
>
> +#ifdef CONFIG_FLATMEM
>         free_area_init(zones_size);
> -}
> -
> -static inline int page_is_ram(unsigned long pagenr)
> -{
> -       int i;
> -
> -       for (i = 0; i < boot_mem_map.nr_map; i++) {
> -               unsigned long addr, end;
> -
> -               if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> -                       /* not usable memory */
> -                       continue;
> -
> -               addr = PFN_UP(boot_mem_map.map[i].addr);
> -               end = PFN_DOWN(boot_mem_map.map[i].addr +
> -                              boot_mem_map.map[i].size);
> -
> -               if (pagenr >= addr && pagenr < end)
> -                       return 1;
> -       }
> -
> -       return 0;
> +#else
> +       pfn = 0;
> +       for (i = 0; i < MAX_NR_ZONES; i++)
> +               for (j = 0; j < zones_size[i]; j++, pfn++)
> +                       if (!page_is_ram(pfn))

can we use pfn_valid() instead of page_is_ram() ? bootmem_init() and
sparse_init() have already been called so pfn_valid() should be safe
here....

> +                               zholes_size[i]++;
> +       free_area_init_node(0, NODE_DATA(0), zones_size, 0, zholes_size);
> +#endif
>  }
>
>  static struct kcore_list kcore_mem, kcore_vmalloc;
> @@ -213,9 +226,9 @@ #ifdef CONFIG_HIGHMEM
>  #ifdef CONFIG_DISCONTIGMEM
>  #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"
>  #endif
> -       max_mapnr = num_physpages = highend_pfn;
> +       max_mapnr = highend_pfn;
>  #else
> -       max_mapnr = num_physpages = max_low_pfn;
> +       max_mapnr = max_low_pfn;

this is not always true, specially if FLATMEM set and your physical mem
do not start at 0.

>  #endif
>         high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT);
>
> @@ -229,6 +242,7 @@ #endif

in this loop can we use pfn_valid() instead of page_is_ram() ? Therefore
we could get rid of the latter macro ?

>                         if (PageReserved(pfn_to_page(tmp)))
>                                 reservedpages++;
>                 }
> +       num_physpages = ram;
>
>  #ifdef CONFIG_HIGHMEM
>         for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
> @@ -247,6 +261,7 @@ #endif
>                 totalhigh_pages++;
>         }
>         totalram_pages += totalhigh_pages;
> +       num_physpages += totalhigh_pages;
>  #endif
>
>         codesize =  (unsigned long) &_etext - (unsigned long) &_text;
>

-- 
               Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Mon, 10 Jul 2006 13:34:06 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> could you put the diffstat next time ?

Sure.  I'll do it.

> can we use pfn_valid() instead of page_is_ram() ? bootmem_init() and
> sparse_init() have already been called so pfn_valid() should be safe
> here....

We can, but we can get more precise value using page_is_ram().  The
pfn_valid() returns true for _all_ pages on present section, and
currently the section size is 256MB.

> > -       max_mapnr = num_physpages = highend_pfn;
> > +       max_mapnr = highend_pfn;
> >  #else
> > -       max_mapnr = num_physpages = max_low_pfn;
> > +       max_mapnr = max_low_pfn;
> 
> this is not always true, specially if FLATMEM set and your physical mem
> do not start at 0.

Yes, and I think you are preparing a patch for these systems ;-)

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Atsushi Nemoto wrote:
> On Mon, 10 Jul 2006 13:34:06 +0200, "Franck Bui-Huu" <vagabon.xyz@gmail.com> wrote:
> 
>> can we use pfn_valid() instead of page_is_ram() ? bootmem_init() and
>> sparse_init() have already been called so pfn_valid() should be safe
>> here....
> 
> We can, but we can get more precise value using page_is_ram().  The
> pfn_valid() returns true for _all_ pages on present section, and
> currently the section size is 256MB.

so your total pages of RAM in show_mem() is incorrect...

               if (!pfn_valid(pfn))
                        continue;
                page = pfn_to_page(pfn);
                total++;


I don't know SPARSEMEM a lot but is it allowed to have holes inside
a section ? Shouldn't we tune the section size to avoid holes inside
section ?

> 
>>> -       max_mapnr = num_physpages = highend_pfn;
>>> +       max_mapnr = highend_pfn;
>>>  #else
>>> -       max_mapnr = num_physpages = max_low_pfn;
>>> +       max_mapnr = max_low_pfn;
>> this is not always true, specially if FLATMEM set and your physical mem
>> do not start at 0.
> 
> Yes, and I think you are preparing a patch for these systems ;-)
> 

good point :)

		Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Tue, 11 Jul 2006 10:33:31 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> > We can, but we can get more precise value using page_is_ram().  The
> > pfn_valid() returns true for _all_ pages on present section, and
> > currently the section size is 256MB.
> 
> so your total pages of RAM in show_mem() is incorrect...
> 
>                if (!pfn_valid(pfn))
>                         continue;
>                 page = pfn_to_page(pfn);
>                 total++;
> 
> 
> I don't know SPARSEMEM a lot but is it allowed to have holes inside
> a section ? Shouldn't we tune the section size to avoid holes inside
> section ?

If holes exist in a section, show_mem() will count these pages as
"reserved".  You can count real pages by "total - reserved".

Talking about nr_kernel_pages (calculated by zones_size[] and
zones_holes[]) and num_physpages, these values are used to determine
sizes of some kernel data structures, it would be better to set more
precise value for them.

While large holes in a section wastes some memory, make the section
size customizable might be a good idea.  Anyone?  ;-)

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Hi Atsushi,

Sorry for the delay...

Atsushi Nemoto wrote:
> On Tue, 11 Jul 2006 10:33:31 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>>> We can, but we can get more precise value using page_is_ram().  The
>>> pfn_valid() returns true for _all_ pages on present section, and
>>> currently the section size is 256MB.
>> so your total pages of RAM in show_mem() is incorrect...
>>
>>                if (!pfn_valid(pfn))
>>                         continue;
>>                 page = pfn_to_page(pfn);
>>                 total++;
>>
>>
>> I don't know SPARSEMEM a lot but is it allowed to have holes inside
>> a section ? Shouldn't we tune the section size to avoid holes inside
>> section ?
> 
> If holes exist in a section, show_mem() will count these pages as
> "reserved".  You can count real pages by "total - reserved".
> 

I don't think that's correct to mark them as "reserved". Basicaly
"reserved" means that it belongs to the kernel (code or data), these
holes are not and we will end up to have wrong value as you pointed
out.

Having quick look at sparsemem code, I don't think that it expects
to have holes inside a section, do it ? If so you probably have to
fix up your section size...

> Talking about nr_kernel_pages (calculated by zones_size[] and
> zones_holes[]) and num_physpages, these values are used to determine
> sizes of some kernel data structures, it would be better to set more
> precise value for them.
> 
> While large holes in a section wastes some memory, make the section
> size customizable might be a good idea.  Anyone?  ;-)
> 

hey, you are working in this area, aren't you ? ;)

		Franck

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Wed, 26 Jul 2006 16:33:45 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> I don't think that's correct to mark them as "reserved". Basicaly
> "reserved" means that it belongs to the kernel (code or data), these
> holes are not and we will end up to have wrong value as you pointed
> out.
> 
> Having quick look at sparsemem code, I don't think that it expects
> to have holes inside a section, do it ? If so you probably have to
> fix up your section size...

Yes, for such small holes, sparsemem and flatmem is same.  We can use
smaller section size to save more memory, but I suppose it will be a
bit slower.

> > Talking about nr_kernel_pages (calculated by zones_size[] and
> > zones_holes[]) and num_physpages, these values are used to determine
> > sizes of some kernel data structures, it would be better to set more
> > precise value for them.
> > 
> > While large holes in a section wastes some memory, make the section
> > size customizable might be a good idea.  Anyone?  ;-)
> > 
> 
> hey, you are working in this area, aren't you ? ;)

Well, not actively now since I'm comfort with 256MB section size :-)

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[Franck Bui-Huu]

Atsushi Nemoto wrote:
> On Wed, 26 Jul 2006 16:33:45 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>> I don't think that's correct to mark them as "reserved". Basicaly
>> "reserved" means that it belongs to the kernel (code or data), these
>> holes are not and we will end up to have wrong value as you pointed
>> out.
>>
>> Having quick look at sparsemem code, I don't think that it expects
>> to have holes inside a section, do it ? If so you probably have to
>> fix up your section size...
> 
> Yes, for such small holes, sparsemem and flatmem is same.  We can use
> smaller section size to save more memory, but I suppose it will be a
> bit slower.
> 

I'm suprised that sparsemem code doens't check for holes inside
sections. I would feel really more confortable to use sparsemem if a
check like the following patch exists. We could safely use pfn_valid()
in _any_ cases and if holes exist inside sections then the user have
to fix up its section sizes.

what do you think ?

		Franck

-- >8 --

diff --git a/mm/sparse.c b/mm/sparse.c
index 86c52ab..4c29a13 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -119,6 +119,13 @@ void memory_present(int nid, unsigned lo
 {
 	unsigned long pfn;
 
+	if (start & (PAGES_PER_SECTION-1) || end & (PAGES_PER_SECTION-1)) {
+		printk(KERN_ERR "SPARSEMEM: memory area (%lx-%lx) creates a "
+		       "hole inside a section, fix your SECTION_SIZE_BITS "
+		       "value...\n", start, end);
+		BUG();
+	}
+	
 	start &= PAGE_SECTION_MASK;
 	for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) {
 		unsigned long section = pfn_to_section_nr(pfn);

Re: [PATCH] do not count pages in holes with sparsemem

[Atsushi Nemoto]

On Thu, 27 Jul 2006 11:00:25 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> I'm suprised that sparsemem code doens't check for holes inside
> sections. I would feel really more confortable to use sparsemem if a
> check like the following patch exists. We could safely use pfn_valid()
> in _any_ cases and if holes exist inside sections then the user have
> to fix up its section sizes.
> 
> what do you think ?

I think holes inside a section is not illegal, just a bit ineffective.
So I feel your patch is too strict.

---
Atsushi Nemoto

Re: [PATCH] do not count pages in holes with sparsemem

[girish]

The system I am concerned with, likewise many MIPS based systems have 256MB
RAM + 256MB I/O in lowmem area. Additional RAM is placed at top of this
area, starting or could even be beyond 2000_0000.

My {quick, dirty, naïve} hack to support HIGHMEM in this case, would be to -
    .1 during bootmem_init, (we need EFI), move highstart_pfn as RAM gets
added beyond lowmem area.
    .2 add I/O space into ZONE_NORMAL (marked reserved later on)
    .3 add Non-RAM pages into ZONE_NORMAL (see below #4)
    .4 reset ZONE_NORMAL num. pages to zero, at the end of free_area_init

At the end zone table looks like -
     ZONE_DMA   = 256MB
    ZONE_NORMAL = 0
    ZONE_HIGHMEM= 256MB (additional RAM for example)

Of course with this approach there will be an overhead of reserved pages.
But then again it also has an use. Consider this, MIPS/UML can make use of
these reserved pages of I/O space (of course marking them un-cached) through
kmap for temporary access or through /dev/kmem for process-term access. For
example, the mmap call on /dev/kmem with offset hint of physical address
1000_0000 where all I/O begins, sets vma->vm_pgoff which in turn extracts an
already reserved page for this frame. The user application (driver?) has
access to I/O space.

Is it possible?


On 7/5/06 10:13 PM, "Atsushi Nemoto" <anemo@mba.ocn.ne.jp> wrote:

> With SPARSEMEM, the single node can contains some holes so there might
> be many invalid pages.  For example, with two 256M memory and one 256M
> hole, some variables (num_physpage, totalpages, nr_kernel_pages,
> nr_all_pages, etc.) will indicate that there are 768MB on this system.
> This is not desired because, for example, alloc_large_system_hash()
> allocates too many entries.
> 
> Use free_area_init_node() with counted zholes_size[] instead of
> free_area_init().
> 
> For num_physpages, use number of ram pages instead of max_low_pfn.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index 802bdd3..d41dee5 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -139,10 +139,36 @@ #endif /* CONFIG_HIGHMEM */
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
>  extern void pagetable_init(void);
>  
> +static int __init page_is_ram(unsigned long pagenr)
> +{
> + int i;
> +
> + for (i = 0; i < boot_mem_map.nr_map; i++) {
> +  unsigned long addr, end;
> +
> +  if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> +   /* not usable memory */
> +   continue;
> +
> +  addr = PFN_UP(boot_mem_map.map[i].addr);
> +  end = PFN_DOWN(boot_mem_map.map[i].addr +
> +          boot_mem_map.map[i].size);
> +
> +  if (pagenr >= addr && pagenr < end)
> +   return 1;
> + }
> +
> + return 0;
> +}
> +
>  void __init paging_init(void)
>  {
> - unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
> + unsigned long zones_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
> unsigned long max_dma, high, low;
> +#ifdef CONFIG_SPARSEMEM
> + unsigned long zholes_size[] = { [0 ... MAX_NR_ZONES - 1] = 0 };
> + unsigned long i, j, pfn;
> +#endif
>  
> pagetable_init();
>  
> @@ -174,29 +200,17 @@ #ifdef CONFIG_HIGHMEM
> zones_size[ZONE_HIGHMEM] = high - low;
>  #endif
>  
> +#ifdef CONFIG_SPARSEMEM
> + pfn = 0;
> + for (i = 0; i < MAX_NR_ZONES; i++)
> +  for (j = 0; j < zones_size[i]; j++, pfn++)
> +   if (!page_is_ram(pfn))
> +    zholes_size[i]++;
> + free_area_init_node(0, NODE_DATA(0), zones_size,
> +       __pa(PAGE_OFFSET), zholes_size);
> +#else
> free_area_init(zones_size);
> -}
> -
> -static inline int page_is_ram(unsigned long pagenr)
> -{
> - int i;
> -
> - for (i = 0; i < boot_mem_map.nr_map; i++) {
> -  unsigned long addr, end;
> -
> -  if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
> -   /* not usable memory */
> -   continue;
> -
> -  addr = PFN_UP(boot_mem_map.map[i].addr);
> -  end = PFN_DOWN(boot_mem_map.map[i].addr +
> -          boot_mem_map.map[i].size);
> -
> -  if (pagenr >= addr && pagenr < end)
> -   return 1;
> - }
> -
> - return 0;
> +#endif
>  }
>  
>  static struct kcore_list kcore_mem, kcore_vmalloc;
> @@ -213,9 +227,9 @@ #ifdef CONFIG_HIGHMEM
>  #ifdef CONFIG_DISCONTIGMEM
>  #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"
>  #endif
> - max_mapnr = num_physpages = highend_pfn;
> + max_mapnr = highend_pfn;
>  #else
> - max_mapnr = num_physpages = max_low_pfn;
> + max_mapnr = max_low_pfn;
>  #endif
> high_memory = (void *) __va(max_low_pfn << PAGE_SHIFT);
>  
> @@ -229,6 +243,7 @@ #endif
> if (PageReserved(pfn_to_page(tmp)))
> reservedpages++;
> }
> + num_physpages = ram;
>  
>  #ifdef CONFIG_HIGHMEM
> for (tmp = highstart_pfn; tmp < highend_pfn; tmp++) {
> @@ -247,6 +262,7 @@ #endif
> totalhigh_pages++;
> }
> totalram_pages += totalhigh_pages;
> + num_physpages += totalhigh_pages;
>  #endif
>  
> codesize =  (unsigned long) &_etext - (unsigned long) &_text;
> 


-- 
First of all - I am an Engineer. I care less for Copyrights/Patents, at
least I have none of my own! I love software development & it pays me to run
my family. I try to dedicate some time thinking about Open Source movement &
sometime contributing to it actually. I often get paid by claiming knowledge
in software developed by Open Source community. Lots of things I know today
& still learning are due to Open Source community.