[PATCH] delete unnecessary bootmem struct page array

[PATCH] delete unnecessary bootmem struct page array

[Real Name]

From: Honggang Li <enjoymindful@gmail.com>

The patch based on linux-next-20140530.

Honggang Li (1):
  delete unnecessary bootmem struct page array

 arch/um/kernel/um_arch.c | 6 ------
 1 file changed, 6 deletions(-)

-- 
1.8.3.1

[PATCH] delete unnecessary bootmem struct page array

[Real Name]

From: Honggang Li <enjoymindful@gmail.com>

1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
not the struct page array, so the array is unnecessary.

2) the bootmem struct page array has been pointed by a *local* pointer,
struct page *map, in init_maps function. The array can be accessed only
in init_maps's scope. As a result, uml kernel wastes about 1% of total
memory.

Signed-off-by: Honggang Li <enjoymindful@gmail.com>
---
 arch/um/kernel/um_arch.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 6043c76..5aa2d82 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -338,12 +338,6 @@ int __init linux_main(int argc, char **argv)
 	start_vm = VMALLOC_START;
 
 	setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
-	if (init_maps(physmem_size, iomem_size, highmem)) {
-		printf("Failed to allocate mem_map for %Lu bytes of physical "
-		       "memory and %Lu bytes of highmem\n", physmem_size,
-		       highmem);
-		exit(1);
-	}
 
 	virtmem_size = physmem_size;
 	stack = (unsigned long) argv;
-- 
1.8.3.1

Re: [PATCH] delete unnecessary bootmem struct page array

[Richard Weinberger]

Am 01.06.2014 15:08, schrieb Real Name:
> From: Honggang Li <enjoymindful@gmail.com>
> 
> 1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
> not the struct page array, so the array is unnecessary.
> 
> 2) the bootmem struct page array has been pointed by a *local* pointer,
> struct page *map, in init_maps function. The array can be accessed only
> in init_maps's scope. As a result, uml kernel wastes about 1% of total
> memory.

I reread your patch again.
You missed one important point. init_maps() setups max_mapnr which is used by
virt_addr_valid().

We have to be extremely careful here to not introduce a subtle fuckup.

Thanks,
//richard

[uml-devel] [PATCH v2] delete unnecessary bootmem struct page array

[Real Name]

From: Honggang Li <enjoymindful@gmail.com>

The patch based on linux-next-2014-06-02.

The old init_maps function does two things:
1) allocates and initializes one struct page array for bootmem
2) count the number of total pages

After removed the source code related to the unnecessary array, the name 
'init_maps' is some kind of improper named, as it just count the number of
total page numbers. So, I renamed the function as 'mem_total_pages'.

I tested the patch through repeat reboot the uml kernel many times.
[real@name linux-next]$ make ARCH=um defconfig
[real@name linux-next]$ make ARCH=um linux
[real@name linux-next]$ file linux
linux: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.32, not stripped
[real@name linux-next]$ ./linux ubda=/home/real/linux-next/Fedora20-AMD64-root_fs mem=256m && sync && echo 1
[real@name linux-next]$ ./linux ubda=/home/real/linux-next/Fedora20-AMD64-root_fs mem=256m && sync && echo 2
(repeat reboot the uml kernel many times..)

Honggang Li (1):
  delete unnecessary bootmem struct page array

 arch/um/include/shared/mem_user.h |  2 +-
 arch/um/kernel/physmem.c          | 32 ++++++--------------------------
 arch/um/kernel/um_arch.c          |  7 +------
 3 files changed, 8 insertions(+), 33 deletions(-)

-- 
1.8.3.1


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their 
applications. Written by three acclaimed leaders in the field, 
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[PATCH] delete unnecessary bootmem struct page array

[Real Name]

From: Honggang Li <enjoymindful@gmail.com>

1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
not the struct page array, so the array is unnecessary.

2) the bootmem struct page array has been pointed by a *local* pointer,
struct page *map, in init_maps function. The array can be accessed only
in init_maps's scope. As a result, uml kernel wastes about 1% of total
memory.

Signed-off-by: Honggang Li <enjoymindful@gmail.com>
---
 arch/um/include/shared/mem_user.h |  2 +-
 arch/um/kernel/physmem.c          | 32 ++++++--------------------------
 arch/um/kernel/um_arch.c          |  7 +------
 3 files changed, 8 insertions(+), 33 deletions(-)

diff --git a/arch/um/include/shared/mem_user.h b/arch/um/include/shared/mem_user.h
index 46384ac..cb84414 100644
--- a/arch/um/include/shared/mem_user.h
+++ b/arch/um/include/shared/mem_user.h
@@ -49,7 +49,7 @@ extern int iomem_size;
 extern int init_mem_user(void);
 extern void setup_memory(void *entry);
 extern unsigned long find_iomem(char *driver, unsigned long *len_out);
-extern int init_maps(unsigned long physmem, unsigned long iomem,
+extern void mem_total_pages(unsigned long physmem, unsigned long iomem,
 		     unsigned long highmem);
 extern unsigned long get_vm(unsigned long len);
 extern void setup_physmem(unsigned long start, unsigned long usable,
diff --git a/arch/um/kernel/physmem.c b/arch/um/kernel/physmem.c
index 30fdd5d..549ecf3 100644
--- a/arch/um/kernel/physmem.c
+++ b/arch/um/kernel/physmem.c
@@ -22,39 +22,19 @@ EXPORT_SYMBOL(high_physmem);
 
 extern unsigned long long physmem_size;
 
-int __init init_maps(unsigned long physmem, unsigned long iomem,
+void __init mem_total_pages(unsigned long physmem, unsigned long iomem,
 		     unsigned long highmem)
 {
-	struct page *p, *map;
-	unsigned long phys_len, phys_pages, highmem_len, highmem_pages;
-	unsigned long iomem_len, iomem_pages, total_len, total_pages;
-	int i;
-
-	phys_pages = physmem >> PAGE_SHIFT;
-	phys_len = phys_pages * sizeof(struct page);
-
-	iomem_pages = iomem >> PAGE_SHIFT;
-	iomem_len = iomem_pages * sizeof(struct page);
+	unsigned long phys_pages, highmem_pages;
+	unsigned long iomem_pages, total_pages;
 
+	phys_pages    = physmem >> PAGE_SHIFT;
+	iomem_pages   = iomem   >> PAGE_SHIFT;
 	highmem_pages = highmem >> PAGE_SHIFT;
-	highmem_len = highmem_pages * sizeof(struct page);
-
-	total_pages = phys_pages + iomem_pages + highmem_pages;
-	total_len = phys_len + iomem_len + highmem_len;
 
-	map = alloc_bootmem_low_pages(total_len);
-	if (map == NULL)
-		return -ENOMEM;
-
-	for (i = 0; i < total_pages; i++) {
-		p = &map[i];
-		memset(p, 0, sizeof(struct page));
-		SetPageReserved(p);
-		INIT_LIST_HEAD(&p->lru);
-	}
+	total_pages   = phys_pages + iomem_pages + highmem_pages;
 
 	max_mapnr = total_pages;
-	return 0;
 }
 
 void map_memory(unsigned long virt, unsigned long phys, unsigned long len,
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 6043c76..dbd5bda 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -338,12 +338,7 @@ int __init linux_main(int argc, char **argv)
 	start_vm = VMALLOC_START;
 
 	setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
-	if (init_maps(physmem_size, iomem_size, highmem)) {
-		printf("Failed to allocate mem_map for %Lu bytes of physical "
-		       "memory and %Lu bytes of highmem\n", physmem_size,
-		       highmem);
-		exit(1);
-	}
+	mem_total_pages(physmem_size, iomem_size, highmem);
 
 	virtmem_size = physmem_size;
 	stack = (unsigned long) argv;
-- 
1.8.3.1

[PATCH] delete unnecessary bootmem struct page array

[Real Name]

1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
not struct page array, so it is unnecessary.

2) the struct page array allocate has been pointer by a *loacl* pointer
struct page *map in init_maps function. The array can't be access after
the init_maps exit. As a result, there is about 1% of total memory leak.
---
 arch/um/kernel/um_arch.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 016adf0..d4c98d1 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -348,12 +348,6 @@ int __init linux_main(int argc, char **argv)
 	start_vm = VMALLOC_START;
 
 	setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
-	if (init_maps(physmem_size, iomem_size, highmem)) {
-		printf("Failed to allocate mem_map for %Lu bytes of physical "
-		       "memory and %Lu bytes of highmem\n", physmem_size,
-		       highmem);
-		exit(1);
-	}
 
 	virtmem_size = physmem_size;
 	stack = (unsigned long) argv;
-- 
1.8.3.1

Re: [PATCH] delete unnecessary bootmem struct page array

[Richard Weinberger]

Hi!

Am 01.06.2014 02:24, schrieb Real Name:
> 1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
> not struct page array, so it is unnecessary.
> 
> 2) the struct page array allocate has been pointer by a *loacl* pointer
> struct page *map in init_maps function. The array can't be access after
> the init_maps exit. As a result, there is about 1% of total memory leak.

Please add a Signed-off-by tag using your real name.

Thanks,
//richard

> ---
>  arch/um/kernel/um_arch.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
> index 016adf0..d4c98d1 100644
> --- a/arch/um/kernel/um_arch.c
> +++ b/arch/um/kernel/um_arch.c
> @@ -348,12 +348,6 @@ int __init linux_main(int argc, char **argv)
>  	start_vm = VMALLOC_START;
>  
>  	setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
> -	if (init_maps(physmem_size, iomem_size, highmem)) {
> -		printf("Failed to allocate mem_map for %Lu bytes of physical "
> -		       "memory and %Lu bytes of highmem\n", physmem_size,
> -		       highmem);
> -		exit(1);
> -	}
>  
>  	virtmem_size = physmem_size;
>  	stack = (unsigned long) argv;
> 

[PATCH] delete unnecessary bootmem struct page array

[Real Name]

1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
not struct page array, so it is unnecessary.

2) the struct page array allocate has been pointer by a *loacl* pointer
struct page *map in init_maps function. The array can't be access after
the init_maps exit. As a result, there is about 1% of total memory leak.
---
 arch/um/kernel/um_arch.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index 016adf0..9f3c6d1 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -309,11 +309,6 @@ int __init linux_main(int argc, char **argv)
 	 */
 
 	diff = UML_ROUND_UP(brk_start) - UML_ROUND_UP(&_end);
-	if (diff > 1024 * 1024) {
-		printf("Adding %ld bytes to physical memory to account for "
-		       "exec-shield gap\n", diff);
-		physmem_size += UML_ROUND_UP(brk_start) - UML_ROUND_UP(&_end);
-	}
 
 	uml_physmem = (unsigned long) &__binary_start & PAGE_MASK;
 
-- 
1.8.3.1

Re: [PATCH] delete unnecessary bootmem struct page array

[Real Name]

On Sun, Jun 01, 2014 at 07:34:42AM +0800, Real Name wrote:

I'm very sorry. Please ignore this email, the wrong patch had been sent
out.

> 1) uml kernel bootmem managed through bootmem_data->node_bootmem_map,
> not struct page array, so it is unnecessary.
> 
> 2) the struct page array allocate has been pointer by a *loacl* pointer
> struct page *map in init_maps function. The array can't be access after
> the init_maps exit. As a result, there is about 1% of total memory leak.
> ---
>  arch/um/kernel/um_arch.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
> index 016adf0..9f3c6d1 100644
> --- a/arch/um/kernel/um_arch.c
> +++ b/arch/um/kernel/um_arch.c
> @@ -309,11 +309,6 @@ int __init linux_main(int argc, char **argv)
>  	 */
>  
>  	diff = UML_ROUND_UP(brk_start) - UML_ROUND_UP(&_end);
> -	if (diff > 1024 * 1024) {
> -		printf("Adding %ld bytes to physical memory to account for "
> -		       "exec-shield gap\n", diff);
> -		physmem_size += UML_ROUND_UP(brk_start) - UML_ROUND_UP(&_end);
> -	}
>  
>  	uml_physmem = (unsigned long) &__binary_start & PAGE_MASK;
>  
> -- 
> 1.8.3.1
>