2005-10-31 03:47:13

by Jeff Dike

[permalink] [raw]
Subject: [PATCH 9/10] UML - Big memory fixes

A number of fixes to improve behavior when large physical memory sizes
are specified:
libc files need -D_FILE_OFFSET_BITS=64 because there are unavoidable
uses of non-64 interfaces in libc
some %d need to be %u

Signed-off-by: Jeff Dike <[email protected]>

Index: linux-2.6.14/arch/um/Makefile
===================================================================
--- linux-2.6.14.orig/arch/um/Makefile 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/Makefile 2005-10-30 19:29:03.000000000 -0500
@@ -60,7 +60,7 @@ AFLAGS += $(ARCH_INCLUDE)

USER_CFLAGS := $(patsubst -I%,,$(CFLAGS))
USER_CFLAGS := $(patsubst -D__KERNEL__,,$(USER_CFLAGS)) $(ARCH_INCLUDE) \
- $(MODE_INCLUDE)
+ $(MODE_INCLUDE) -D_FILE_OFFSET_BITS=64

# -Derrno=kernel_errno - This turns all kernel references to errno into
# kernel_errno to separate them from the libc errno. This allows -fno-common
Index: linux-2.6.14/arch/um/include/mem_user.h
===================================================================
--- linux-2.6.14.orig/arch/um/include/mem_user.h 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/include/mem_user.h 2005-10-30 19:29:04.000000000 -0500
@@ -57,7 +57,7 @@ extern int init_maps(unsigned long physm
unsigned long highmem);
extern unsigned long get_vm(unsigned long len);
extern void setup_physmem(unsigned long start, unsigned long usable,
- unsigned long len, unsigned long highmem);
+ unsigned long len, unsigned long long highmem);
extern void add_iomem(char *name, int fd, unsigned long size);
extern unsigned long phys_offset(unsigned long phys);
extern void unmap_physmem(void);
Index: linux-2.6.14/arch/um/include/os.h
===================================================================
--- linux-2.6.14.orig/arch/um/include/os.h 2005-10-30 19:27:49.000000000 -0500
+++ linux-2.6.14/arch/um/include/os.h 2005-10-30 19:29:04.000000000 -0500
@@ -167,7 +167,7 @@ extern int can_do_skas(void);
#endif

/* mem.c */
-extern int create_mem_file(unsigned long len);
+extern int create_mem_file(unsigned long long len);

/* process.c */
extern unsigned long os_process_pc(int pid);
Index: linux-2.6.14/arch/um/kernel/mem.c
===================================================================
--- linux-2.6.14.orig/arch/um/kernel/mem.c 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/kernel/mem.c 2005-10-30 19:29:04.000000000 -0500
@@ -235,7 +235,7 @@ void paging_init(void)
for(i=0;i<sizeof(zones_size)/sizeof(zones_size[0]);i++)
zones_size[i] = 0;
zones_size[0] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
- zones_size[2] = highmem >> PAGE_SHIFT;
+ zones_size[3] = highmem >> PAGE_SHIFT;
free_area_init(zones_size);

/*
Index: linux-2.6.14/arch/um/kernel/physmem.c
===================================================================
--- linux-2.6.14.orig/arch/um/kernel/physmem.c 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/kernel/physmem.c 2005-10-30 19:29:04.000000000 -0500
@@ -246,7 +246,7 @@ int is_remapped(void *virt)
/* Changed during early boot */
unsigned long high_physmem;

-extern unsigned long physmem_size;
+extern unsigned long long physmem_size;

int init_maps(unsigned long physmem, unsigned long iomem, unsigned long highmem)
{
@@ -321,7 +321,7 @@ void map_memory(unsigned long virt, unsi
extern int __syscall_stub_start, __binary_start;

void setup_physmem(unsigned long start, unsigned long reserve_end,
- unsigned long len, unsigned long highmem)
+ unsigned long len, unsigned long long highmem)
{
unsigned long reserve = reserve_end - start;
int pfn = PFN_UP(__pa(reserve_end));
Index: linux-2.6.14/arch/um/kernel/um_arch.c
===================================================================
--- linux-2.6.14.orig/arch/um/kernel/um_arch.c 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/kernel/um_arch.c 2005-10-30 19:29:04.000000000 -0500
@@ -137,7 +137,7 @@ static char *argv1_end = NULL;

/* Set in early boot */
static int have_root __initdata = 0;
-long physmem_size = 32 * 1024 * 1024;
+long long physmem_size = 32 * 1024 * 1024;

void set_cmdline(char *cmd)
{
@@ -402,7 +402,7 @@ int linux_main(int argc, char **argv)
#ifndef CONFIG_HIGHMEM
highmem = 0;
printf("CONFIG_HIGHMEM not enabled - physical memory shrunk "
- "to %ld bytes\n", physmem_size);
+ "to %lu bytes\n", physmem_size);
#endif
}

@@ -414,8 +414,8 @@ int linux_main(int argc, char **argv)

setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
if(init_maps(physmem_size, iomem_size, highmem)){
- printf("Failed to allocate mem_map for %ld bytes of physical "
- "memory and %ld bytes of highmem\n", physmem_size,
+ printf("Failed to allocate mem_map for %lu bytes of physical "
+ "memory and %lu bytes of highmem\n", physmem_size,
highmem);
exit(1);
}
@@ -426,7 +426,7 @@ int linux_main(int argc, char **argv)
end_vm = start_vm + virtmem_size;

if(virtmem_size < physmem_size)
- printf("Kernel virtual memory size shrunk to %ld bytes\n",
+ printf("Kernel virtual memory size shrunk to %lu bytes\n",
virtmem_size);

uml_postsetup();
Index: linux-2.6.14/arch/um/os-Linux/mem.c
===================================================================
--- linux-2.6.14.orig/arch/um/os-Linux/mem.c 2005-10-28 12:58:12.000000000 -0400
+++ linux-2.6.14/arch/um/os-Linux/mem.c 2005-10-30 19:29:05.000000000 -0500
@@ -88,7 +88,7 @@ int make_tempfile(const char *template,
* This proc is used in start_up.c
* So it isn't 'static'.
*/
-int create_tmp_file(unsigned long len)
+int create_tmp_file(unsigned long long len)
{
int fd, err;
char zero;
@@ -121,7 +121,7 @@ int create_tmp_file(unsigned long len)
return(fd);
}

-static int create_anon_file(unsigned long len)
+static int create_anon_file(unsigned long long len)
{
void *addr;
int fd;
@@ -144,7 +144,7 @@ static int create_anon_file(unsigned lon

extern int have_devanon;

-int create_mem_file(unsigned long len)
+int create_mem_file(unsigned long long len)
{
int err, fd;

Index: linux-2.6.14/arch/um/os-Linux/start_up.c
===================================================================
--- linux-2.6.14.orig/arch/um/os-Linux/start_up.c 2005-10-30 19:28:48.000000000 -0500
+++ linux-2.6.14/arch/um/os-Linux/start_up.c 2005-10-30 19:29:05.000000000 -0500
@@ -296,7 +296,7 @@ static void __init check_ptrace(void)
check_sysemu();
}

-extern int create_tmp_file(unsigned long len);
+extern int create_tmp_file(unsigned long long len);

static void check_tmpexec(void)
{


2005-11-01 15:06:28

by Blaisorblade

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH 9/10] UML - Big memory fixes

On Monday 31 October 2005 05:39, Jeff Dike wrote:
> A number of fixes to improve behavior when large physical memory sizes
> are specified:
> libc files need -D_FILE_OFFSET_BITS=64 because there are unavoidable
> uses of non-64 interfaces in libc
> some %d need to be %u
>
> Signed-off-by: Jeff Dike <[email protected]>

Jeff, please always make patch and ChangeLog match. Yes, it matters - when it
doesn't happen there's always something bad going on.

> Index: linux-2.6.14/arch/um/kernel/mem.c
> ===================================================================
> --- linux-2.6.14.orig/arch/um/kernel/mem.c 2005-10-28 12:58:12.000000000
> -0400 +++ linux-2.6.14/arch/um/kernel/mem.c 2005-10-30 19:29:04.000000000
> -0500 @@ -235,7 +235,7 @@ void paging_init(void)
> for(i=0;i<sizeof(zones_size)/sizeof(zones_size[0]);i++)
> zones_size[i] = 0;
> zones_size[0] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
> - zones_size[2] = highmem >> PAGE_SHIFT;
> + zones_size[3] = highmem >> PAGE_SHIFT;
> free_area_init(zones_size);

What's this? It's IMHO invalid. free_area_init() intereprets these values with
include/linux/mmzone.h: ZONE_* constants.

Why those are not used here it's a nice question.

I think this came up because there's (in -mm) Andi Kleen created a new zone
(ZONE_DMA32) for devices using 32-bit only DMA - but it seems it's not in
mainline). (I don't know if that patch is in -mm actually, but I guess it
from this patch content).

Actually, since MAX_NR_ZONES is 3, and we are assigning to zones_size[3],
we're corrupting data:

unsigned long zones_size[MAX_NR_ZONES], vaddr;

Don't drop the patch however, I'm fixing all this up.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade


___________________________________
Yahoo! Messenger: chiamate gratuite in tutto il mondo
http://it.messenger.yahoo.com

2005-11-01 16:19:11

by Jeff Dike

[permalink] [raw]
Subject: Re: [uml-devel] [PATCH 9/10] UML - Big memory fixes

On Tue, Nov 01, 2005 at 04:11:04PM +0100, Blaisorblade wrote:
> I think this came up because there's (in -mm) Andi Kleen created a new zone
> (ZONE_DMA32) for devices using 32-bit only DMA - but it seems it's not in
> mainline). (I don't know if that patch is in -mm actually, but I guess it
> from this patch content).

Correct, nice spotting.

This chunk leaked from my -mm tree accidentally, so it is appropriate for -mm,
but it should be its own separate -mm-only patch.

Jeff

2005-11-01 20:32:58

by Blaisorblade

[permalink] [raw]
Subject: [PATCH] uml: fix hardcoded ZONE_* constants in zone setup

From: Paolo 'Blaisorblade' Giarrusso <[email protected]>

Remove usage of hardcoded constants in paging_init().

By chance I spotted a bug in zones_setup involving a change to ZONE_* constants,
due to the ZONE_DMA32 patch from Andi Kleen (which is in -mm). So, possibly,
instead of zones_size[2] you will find zones_size[3] in the code, but that
change is wrong and this patch is still correct.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---

arch/um/kernel/mem.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c
--- a/arch/um/kernel/mem.c
+++ b/arch/um/kernel/mem.c
@@ -234,8 +234,8 @@ void paging_init(void)
empty_bad_page = (unsigned long *) alloc_bootmem_low_pages(PAGE_SIZE);
for(i=0;i<sizeof(zones_size)/sizeof(zones_size[0]);i++)
zones_size[i] = 0;
- zones_size[0] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
- zones_size[2] = highmem >> PAGE_SHIFT;
+ zones_size[ZONE_DMA] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
+ zones_size[ZONE_HIGHMEM] = highmem >> PAGE_SHIFT;
free_area_init(zones_size);

/*

2005-11-04 22:46:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] uml: fix hardcoded ZONE_* constants in zone setup

"Paolo 'Blaisorblade' Giarrusso" <[email protected]> wrote:
>
> From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
>
> Remove usage of hardcoded constants in paging_init().
>
> By chance I spotted a bug in zones_setup involving a change to ZONE_* constants,
> due to the ZONE_DMA32 patch from Andi Kleen (which is in -mm). So, possibly,
> instead of zones_size[2] you will find zones_size[3] in the code, but that
> change is wrong and this patch is still correct.
>
> Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
> ---
>
> arch/um/kernel/mem.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c
> --- a/arch/um/kernel/mem.c
> +++ b/arch/um/kernel/mem.c
> @@ -234,8 +234,8 @@ void paging_init(void)
> empty_bad_page = (unsigned long *) alloc_bootmem_low_pages(PAGE_SIZE);
> for(i=0;i<sizeof(zones_size)/sizeof(zones_size[0]);i++)
> zones_size[i] = 0;
> - zones_size[0] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
> - zones_size[2] = highmem >> PAGE_SHIFT;
> + zones_size[ZONE_DMA] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
> + zones_size[ZONE_HIGHMEM] = highmem >> PAGE_SHIFT;

An earlier patch from Jeff (below) already changed this code.

Jeff's change looks rather wrong:



#define MAX_NR_ZONES 3 /* Sync this with ZONES_SHIFT */

unsigned long zones_size[MAX_NR_ZONES]

...

zones_size[3] = highmem >> PAGE_SHIFT;

which overindexes the local array.

The above change is unmentioned in Jeff's changelog and I'll just drop that
part. Please confirm.

There are other parts of this patch whci are unchangelogged. Please
double-check the whole thing.


From: Jeff Dike <[email protected]>

A number of fixes to improve behavior when large physical memory sizes
are specified:

- libc files need -D_FILE_OFFSET_BITS=64 because there are unavoidable uses
of non-64 interfaces in libc

- some %d need to be %u

Signed-off-by: Jeff Dike <[email protected]>
Cc: Paolo Giarrusso <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

arch/um/Makefile | 2 +-
arch/um/include/mem_user.h | 2 +-
arch/um/include/os.h | 2 +-
arch/um/kernel/mem.c | 2 +-
arch/um/kernel/physmem.c | 4 ++--
arch/um/kernel/um_arch.c | 10 +++++-----
arch/um/os-Linux/mem.c | 6 +++---
arch/um/os-Linux/start_up.c | 2 +-
8 files changed, 15 insertions(+), 15 deletions(-)

diff -puN arch/um/include/mem_user.h~uml-big-memory-fixes arch/um/include/mem_user.h
--- devel/arch/um/include/mem_user.h~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/include/mem_user.h 2005-10-31 17:43:54.000000000 -0800
@@ -57,7 +57,7 @@ extern int init_maps(unsigned long physm
unsigned long highmem);
extern unsigned long get_vm(unsigned long len);
extern void setup_physmem(unsigned long start, unsigned long usable,
- unsigned long len, unsigned long highmem);
+ unsigned long len, unsigned long long highmem);
extern void add_iomem(char *name, int fd, unsigned long size);
extern unsigned long phys_offset(unsigned long phys);
extern void unmap_physmem(void);
diff -puN arch/um/include/os.h~uml-big-memory-fixes arch/um/include/os.h
--- devel/arch/um/include/os.h~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/include/os.h 2005-10-31 17:43:54.000000000 -0800
@@ -167,7 +167,7 @@ extern int can_do_skas(void);
#endif

/* mem.c */
-extern int create_mem_file(unsigned long len);
+extern int create_mem_file(unsigned long long len);

/* process.c */
extern unsigned long os_process_pc(int pid);
diff -puN arch/um/kernel/mem.c~uml-big-memory-fixes arch/um/kernel/mem.c
--- devel/arch/um/kernel/mem.c~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/kernel/mem.c 2005-10-31 17:43:54.000000000 -0800
@@ -235,7 +235,7 @@ void paging_init(void)
for(i=0;i<sizeof(zones_size)/sizeof(zones_size[0]);i++)
zones_size[i] = 0;
zones_size[0] = (end_iomem >> PAGE_SHIFT) - (uml_physmem >> PAGE_SHIFT);
- zones_size[2] = highmem >> PAGE_SHIFT;
+ zones_size[3] = highmem >> PAGE_SHIFT;
free_area_init(zones_size);

/*
diff -puN arch/um/kernel/physmem.c~uml-big-memory-fixes arch/um/kernel/physmem.c
--- devel/arch/um/kernel/physmem.c~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/kernel/physmem.c 2005-10-31 17:43:54.000000000 -0800
@@ -246,7 +246,7 @@ int is_remapped(void *virt)
/* Changed during early boot */
unsigned long high_physmem;

-extern unsigned long physmem_size;
+extern unsigned long long physmem_size;

int init_maps(unsigned long physmem, unsigned long iomem, unsigned long highmem)
{
@@ -321,7 +321,7 @@ void map_memory(unsigned long virt, unsi
extern int __syscall_stub_start, __binary_start;

void setup_physmem(unsigned long start, unsigned long reserve_end,
- unsigned long len, unsigned long highmem)
+ unsigned long len, unsigned long long highmem)
{
unsigned long reserve = reserve_end - start;
int pfn = PFN_UP(__pa(reserve_end));
diff -puN arch/um/kernel/um_arch.c~uml-big-memory-fixes arch/um/kernel/um_arch.c
--- devel/arch/um/kernel/um_arch.c~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/kernel/um_arch.c 2005-10-31 17:43:54.000000000 -0800
@@ -137,7 +137,7 @@ static char *argv1_end = NULL;

/* Set in early boot */
static int have_root __initdata = 0;
-long physmem_size = 32 * 1024 * 1024;
+long long physmem_size = 32 * 1024 * 1024;

void set_cmdline(char *cmd)
{
@@ -402,7 +402,7 @@ int linux_main(int argc, char **argv)
#ifndef CONFIG_HIGHMEM
highmem = 0;
printf("CONFIG_HIGHMEM not enabled - physical memory shrunk "
- "to %ld bytes\n", physmem_size);
+ "to %lu bytes\n", physmem_size);
#endif
}

@@ -414,8 +414,8 @@ int linux_main(int argc, char **argv)

setup_physmem(uml_physmem, uml_reserved, physmem_size, highmem);
if(init_maps(physmem_size, iomem_size, highmem)){
- printf("Failed to allocate mem_map for %ld bytes of physical "
- "memory and %ld bytes of highmem\n", physmem_size,
+ printf("Failed to allocate mem_map for %lu bytes of physical "
+ "memory and %lu bytes of highmem\n", physmem_size,
highmem);
exit(1);
}
@@ -426,7 +426,7 @@ int linux_main(int argc, char **argv)
end_vm = start_vm + virtmem_size;

if(virtmem_size < physmem_size)
- printf("Kernel virtual memory size shrunk to %ld bytes\n",
+ printf("Kernel virtual memory size shrunk to %lu bytes\n",
virtmem_size);

uml_postsetup();
diff -puN arch/um/Makefile~uml-big-memory-fixes arch/um/Makefile
--- devel/arch/um/Makefile~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/Makefile 2005-10-31 17:43:54.000000000 -0800
@@ -60,7 +60,7 @@ AFLAGS += $(ARCH_INCLUDE)

USER_CFLAGS := $(patsubst -I%,,$(CFLAGS))
USER_CFLAGS := $(patsubst -D__KERNEL__,,$(USER_CFLAGS)) $(ARCH_INCLUDE) \
- $(MODE_INCLUDE)
+ $(MODE_INCLUDE) -D_FILE_OFFSET_BITS=64

# -Derrno=kernel_errno - This turns all kernel references to errno into
# kernel_errno to separate them from the libc errno. This allows -fno-common
diff -puN arch/um/os-Linux/mem.c~uml-big-memory-fixes arch/um/os-Linux/mem.c
--- devel/arch/um/os-Linux/mem.c~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/os-Linux/mem.c 2005-10-31 17:43:54.000000000 -0800
@@ -88,7 +88,7 @@ int make_tempfile(const char *template,
* This proc is used in start_up.c
* So it isn't 'static'.
*/
-int create_tmp_file(unsigned long len)
+int create_tmp_file(unsigned long long len)
{
int fd, err;
char zero;
@@ -121,7 +121,7 @@ int create_tmp_file(unsigned long len)
return(fd);
}

-static int create_anon_file(unsigned long len)
+static int create_anon_file(unsigned long long len)
{
void *addr;
int fd;
@@ -144,7 +144,7 @@ static int create_anon_file(unsigned lon

extern int have_devanon;

-int create_mem_file(unsigned long len)
+int create_mem_file(unsigned long long len)
{
int err, fd;

diff -puN arch/um/os-Linux/start_up.c~uml-big-memory-fixes arch/um/os-Linux/start_up.c
--- devel/arch/um/os-Linux/start_up.c~uml-big-memory-fixes 2005-10-31 17:43:54.000000000 -0800
+++ devel-akpm/arch/um/os-Linux/start_up.c 2005-10-31 17:43:54.000000000 -0800
@@ -296,7 +296,7 @@ static void __init check_ptrace(void)
check_sysemu();
}

-extern int create_tmp_file(unsigned long len);
+extern int create_tmp_file(unsigned long long len);

static void check_tmpexec(void)
{
_

2005-11-04 23:16:35

by Blaisorblade

[permalink] [raw]
Subject: Re: [PATCH] uml: fix hardcoded ZONE_* constants in zone setup

On Friday 04 November 2005 23:46, Andrew Morton wrote:
> "Paolo 'Blaisorblade' Giarrusso" <[email protected]> wrote:
> > From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
> >
> > Remove usage of hardcoded constants in paging_init().

> An earlier patch from Jeff (below) already changed this code.

Andrew, yes indeed: quoting from my changelog (yep, I should have made it
clearer):
> > By chance I spotted a bug in zones_setup involving a change to ZONE_*
> > constants, due to the ZONE_DMA32 patch from Andi Kleen (which is in -mm).
> > So, possibly, instead of zones_size[2] you will find zones_size[3] in the
> > code, but that change is wrong and this patch is still correct.
I'm talking exactly of this Jeff's change, and I did exactly what you did...

Thanks anyway for spending some time caring about this, it's nice to see
attention on UML (sorry, no kidding and no complaining).

> Jeff's change looks rather wrong:
The original reason was done for -mm and ZONE_DMA32.

> #define MAX_NR_ZONES 3 /* Sync this with ZONES_SHIFT */
> zones_size[3] = highmem >> PAGE_SHIFT;
>
> which overindexes the local array.

> The above change is unmentioned in Jeff's changelog and I'll just drop that
> part. Please confirm.
Yep, ACK.
> There are other parts of this patch whci are unchangelogged. Please
> double-check the whole thing.

I will re-check again later, but the unchangelogged changes seem to be just
long -> long long conversions.
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade





___________________________________
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB
http://mail.yahoo.it

2005-11-04 23:26:07

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] uml: fix hardcoded ZONE_* constants in zone setup

Blaisorblade <[email protected]> wrote:
>
> On Friday 04 November 2005 23:46, Andrew Morton wrote:
> > "Paolo 'Blaisorblade' Giarrusso" <[email protected]> wrote:
> > > From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
> > >
> > > Remove usage of hardcoded constants in paging_init().
>
> > An earlier patch from Jeff (below) already changed this code.
>
> Andrew, yes indeed: quoting from my changelog (yep, I should have made it
> clearer):
> > > By chance I spotted a bug in zones_setup involving a change to ZONE_*
> > > constants, due to the ZONE_DMA32 patch from Andi Kleen (which is in -mm).
> > > So, possibly, instead of zones_size[2] you will find zones_size[3] in the
> > > code, but that change is wrong and this patch is still correct.
> I'm talking exactly of this Jeff's change, and I did exactly what you did...

It all comes back to me now ;)

> Thanks anyway for spending some time caring about this, it's nice to see
> attention on UML (sorry, no kidding and no complaining).
>
> > Jeff's change looks rather wrong:
> The original reason was done for -mm and ZONE_DMA32.
>
> > #define MAX_NR_ZONES 3 /* Sync this with ZONES_SHIFT */
> > zones_size[3] = highmem >> PAGE_SHIFT;
> >
> > which overindexes the local array.
>
> > The above change is unmentioned in Jeff's changelog and I'll just drop that
> > part. Please confirm.
> Yep, ACK.

OK, using ZONE_HIGHMEM there will insulate UML from Andi's changes.

> > There are other parts of this patch whci are unchangelogged. Please
> > double-check the whole thing.
>
> I will re-check again later, but the unchangelogged changes seem to be just
> long -> long long conversions.

OK, thanks.