[BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[Coywolf Qi Hunt]

<http://localhost/lxr/ident?i=start_kernel>Hello all,

On machine with 16G(or 8G if 4k stacks) or more memory, high max_threads 
could let system run out of low memory.
This patch decides max_threads by the amount of low memory instead of 
the total physical memory.
Systems without high memory would not be affected.

        Coywolf

=====================================================================
diff -rup linux-2.6.7/init/main.c linux-2.6.7-cy/init/main.c
--- linux-2.6.7/init/main.c	Wed Jun  9 00:07:40 2004
+++ linux-2.6.7-cy/init/main.c	Thu Jun 17 04:55:54 2004
@@ -467,7 +467,7 @@ asmlinkage void __init start_kernel(void
 	if (efi_enabled)
 		efi_enter_virtual_mode();
 #endif
-	fork_init(num_physpages);
+	fork_init(max_low_pfn);
 	proc_caches_init();
 	buffer_init();
 	unnamed_dev_init();
diff -rup linux-2.6.7/kernel/fork.c linux-2.6.7-cy/kernel/fork.c
--- linux-2.6.7/kernel/fork.c	Wed Jun  9 00:07:40 2004
+++ linux-2.6.7-cy/kernel/fork.c	Mon Jun 28 22:55:50 2004
@@ -224,8 +224,8 @@ void __init fork_init(unsigned long memp
 
 	/*
 	 * The default maximum number of threads is set to a safe
-	 * value: the thread structures can take up at most half
-	 * of memory.
+	 * value: the thread structures can take up at most 1/8
+	 * of low memory.
 	 */
 	max_threads = mempages / (THREAD_SIZE/PAGE_SIZE) / 8;
 	/*

-- 
Coywolf Qi Hunt
Admin of http://GreatCN.org and http://LoveCN.org

Re: [BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[Russell King]

On Mon, Jun 28, 2004 at 11:55:29PM +0800, Coywolf Qi Hunt wrote:
> <http://localhost/lxr/ident?i=start_kernel>Hello all,
> 
> On machine with 16G(or 8G if 4k stacks) or more memory, high max_threads 
> could let system run out of low memory.
> This patch decides max_threads by the amount of low memory instead of 
> the total physical memory.
> Systems without high memory would not be affected.

This is wrong - max_low_pfn can be high on systems where physical RAM
doesn't start at address 0.  Such is very common on ARM platforms,
where RAM is located at 0xa0000000 or 0xc0000000 physical, which
leads to any calculation based upon max_low_pfn to believe we have
more than 3GB of RAM when we may only have 64MB or so.

I think we may need a num_lowpages for this...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[Coywolf Qi Hunt]

Russell King wrote:

>On Mon, Jun 28, 2004 at 11:55:29PM +0800, Coywolf Qi Hunt wrote:
>  
>
>>Hello all,
>>
>>On machine with 16G(or 8G if 4k stacks) or more memory, high max_threads 
>>could let system run out of low memory.
>>This patch decides max_threads by the amount of low memory instead of 
>>the total physical memory.
>>Systems without high memory would not be affected.
>>    
>>
>
>This is wrong - max_low_pfn can be high on systems where physical RAM
>doesn't start at address 0.  Such is very common on ARM platforms,
>where RAM is located at 0xa0000000 or 0xc0000000 physical, which
>leads to any calculation based upon max_low_pfn to believe we have
>more than 3GB of RAM when we may only have 64MB or so.
>
>I think we may need a num_lowpages for this...
>
Actually there's physical DRAM offset: PHY_OFFSET, defined on ARM only. 
max_low_pfn happens to be the same as `num_lowpages'.
These assignments seems illogical in naming. But just happen to let this 
patch work.  Other platforms may still break.

[coywolf@everest ~/linux-2.6.7/arch]$ grep max_low_pfn arm* -rn
arm/mm/init.c:235:      max_low_pfn = memend_pfn - O_PFN_DOWN(PHYS_OFFSET);
arm26/mm/init.c:187:    max_low_pfn = memend_pfn - PFN_DOWN(PHYS_OFFSET);
arm26/mm/mm-memc.c:157: page_nr = max_low_pfn;

-- 
Coywolf Qi Hunt
Admin of http://GreatCN.org and http://LoveCN.org

Re: [BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[Russell King]

On Tue, Jun 29, 2004 at 06:48:14PM +0800, Coywolf Qi Hunt wrote:
> Russell King wrote:
> Actually there's physical DRAM offset: PHY_OFFSET, defined on ARM only. 
> max_low_pfn happens to be the same as `num_lowpages'.
> These assignments seems illogical in naming. But just happen to let this 
> patch work.  Other platforms may still break.

That may be a bug actually.  Looking at ll_rw_blk.c:

        unsigned long bounce_pfn = dma_addr >> PAGE_SHIFT;
        if (bounce_pfn < blk_max_low_pfn) {

        blk_max_low_pfn = max_low_pfn;

dma_addr are physical addresses, so bounce_pfn is referenced to a PFN0
equal to physical address 0.  This implies that blk_max_low_pfn is
likewise, as is max_low_pfn.

> [coywolf@everest ~/linux-2.6.7/arch]$ grep max_low_pfn arm* -rn
> arm/mm/init.c:235:      max_low_pfn = memend_pfn - O_PFN_DOWN(PHYS_OFFSET);

However, here, max_low_pfn of zero corresponds with the PFN of
PHYS_OFFSET.  We have something with two different origins being
compared, which is nonsense.  So something is wrong somewhere,
and my money is on max_low_pfn.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

[BUG FIX] [ARM/ARM26] find_memend_and_nodes bug fix

[Coywolf Qi Hunt]

Russell King wrote:

>On Tue, Jun 29, 2004 at 06:48:14PM +0800, Coywolf Qi Hunt wrote:
>  
>
>>Russell King wrote:
>>Actually there's physical DRAM offset: PHY_OFFSET, defined on ARM only. 
>>max_low_pfn happens to be the same as `num_lowpages'.
>>These assignments seems illogical in naming. But just happen to let this 
>>patch work.  Other platforms may still break.
>>    
>>
>
>That may be a bug actually.  Looking at ll_rw_blk.c:
>
>        unsigned long bounce_pfn = dma_addr >> PAGE_SHIFT;
>        if (bounce_pfn < blk_max_low_pfn) {
>
>        blk_max_low_pfn = max_low_pfn;
>
>dma_addr are physical addresses, so bounce_pfn is referenced to a PFN0
>equal to physical address 0.  This implies that blk_max_low_pfn is
>likewise, as is max_low_pfn.
>
>  
>
>>[coywolf@everest ~/linux-2.6.7/arch]$ grep max_low_pfn arm* -rn
>>arm/mm/init.c:235:      max_low_pfn = memend_pfn - O_PFN_DOWN(PHYS_OFFSET);
>>    
>>
>
>However, here, max_low_pfn of zero corresponds with the PFN of
>PHYS_OFFSET.  We have something with two different origins being
>compared, which is nonsense.  So something is wrong somewhere,
>and my money is on max_low_pfn.
>
>  
>
The bug may get into panic when there's still enough memory for block i/o.
Here's the patch with also a BUG_ON improvement.


=======================================================================

diff -Nrup linux-2.6.7/arch/arm/mm/init.c linux-2.6.7-cy2/arch/arm/mm/init.c
--- linux-2.6.7/arch/arm/mm/init.c	2004-06-29 23:03:30.000000000 -0500
+++ linux-2.6.7-cy2/arch/arm/mm/init.c	2004-06-30 04:32:42.215999091 -0500
@@ -231,9 +231,10 @@ find_memend_and_nodes(struct meminfo *mi
 	 * This doesn't seem to be used by the Linux memory
 	 * manager any more.  If we can get rid of it, we
 	 * also get rid of some of the stuff above as well.
+	 *
+	 * blk_max_low_pfn depends on this. -- coywolf
 	 */
-	max_low_pfn = memend_pfn - O_PFN_DOWN(PHYS_OFFSET);
-	max_pfn = memend_pfn - O_PFN_DOWN(PHYS_OFFSET);
+	max_low_pfn = max_pfn = memend_pfn;
 
 	return bootmem_pages;
 }
diff -Nrup linux-2.6.7/arch/arm26/mm/init.c linux-2.6.7-cy2/arch/arm26/mm/init.c
--- linux-2.6.7/arch/arm26/mm/init.c	2004-05-09 21:33:20.000000000 -0500
+++ linux-2.6.7-cy2/arch/arm26/mm/init.c	2004-06-30 03:59:51.000000000 -0500
@@ -160,9 +160,7 @@ find_memend_and_nodes(struct meminfo *mi
 
 	np->bootmap_pages = 0;
 
-	if (mi->bank->size == 0) {
-		BUG();
-	}
+	BUG_ON(mi->bank->size == 0)
 
 	/*
 	 * Get the start and end pfns for this bank
@@ -183,9 +181,10 @@ find_memend_and_nodes(struct meminfo *mi
 	 * This doesn't seem to be used by the Linux memory
 	 * manager any more.  If we can get rid of it, we
 	 * also get rid of some of the stuff above as well.
+	 *
+	 * blk_max_low_pfn depends on this. -- coywolf
 	 */
-	max_low_pfn = memend_pfn - PFN_DOWN(PHYS_OFFSET);
-	max_pfn = memend_pfn - PFN_DOWN(PHYS_OFFSET);
+	max_low_pfn = max_pfn = memend_pfn;
 	mi->end = memend_pfn << PAGE_SHIFT;
 
 }

-- 

Coywolf Qi Hunt
Admin of http://GreatCN.org and http://LoveCN.org

Re: [BUG FIX] [PATCH] fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

[Chris Wedgwood]

On Mon, Jun 28, 2004 at 05:53:25PM +0100, Russell King wrote:

> This is wrong - max_low_pfn can be high on systems where physical
> RAM doesn't start at address 0.

FWIW sn2 also doesn't have memory at 0 either.


  --cw

[BUG FIX] fork_init() OOM bug on big highmem machine

[Coywolf Qi Hunt]

Coywolf Qi Hunt wrote:

> Hello all,
>
> On machine with 16G(or 8G if 4k stacks) or more memory, high 
> max_threads could let system run out of low memory.
> This patch decides max_threads by the amount of low memory instead of 
> the total physical memory.
> Systems without high memory would not be affected. 

This patch should be ok by taking ``max_low_pfn - min_low_pfn'' and also 
becomes more accurate.

On those systems where physical RAM doesn't start at address 0, we 
should initialize min_low_pfn.
Anyway min_low_pfn is already defined on all platforms. The bug of 
forgetting min_low_pfn initialization
on those platforms isn't more severe than this bug.

====================================================================

diff -rup linux-2.6.7/init/main.c linux-2.6.7-cy/init/main.c
--- linux-2.6.7/init/main.c	Wed Jun  9 00:07:40 2004
+++ linux-2.6.7-cy/init/main.c	Thu Jun 17 04:55:54 2004
@@ -467,7 +467,7 @@ asmlinkage void __init start_kernel(void
 	if (efi_enabled)
 		efi_enter_virtual_mode();
 #endif
-	fork_init(num_physpages);
+	fork_init(max_low_pfn - min_low_pfn);
 	proc_caches_init();
 	buffer_init();
 	unnamed_dev_init();
diff -rup linux-2.6.7/kernel/fork.c linux-2.6.7-cy/kernel/fork.c
--- linux-2.6.7/kernel/fork.c	Wed Jun  9 00:07:40 2004
+++ linux-2.6.7-cy/kernel/fork.c	Mon Jun 28 22:55:50 2004
@@ -224,8 +224,8 @@ void __init fork_init(unsigned long memp
 
 	/*
 	 * The default maximum number of threads is set to a safe
-	 * value: the thread structures can take up at most half
-	 * of memory.
+	 * value: the thread structures can take up at most 1/8
+	 * of low memory.
 	 */
 	max_threads = mempages / (THREAD_SIZE/PAGE_SIZE) / 8;
 	/*


-- 
Coywolf Qi Hunt
Admin of http://GreatCN.org and http://LoveCN.org