fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

PATCH In review Coywolf Qi Hunt 3 participants 7 messages Jun 28, 2004 – Jun 30, 2004

Subsystems (4)

Arm (32-bit) MM core Scheduler exec and binfmt

lore

Share & tools

lore

Coywolf Qi Hunt

author

2004-06-28 15:55 UTC #0 lore raw

PATCH 1/1 fork_init() max_low_pfn fixes potential OOM bug on big highmem machine

<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

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

init/main.c +0 -0

diff -rup linux-2.6.7/init/main.c linux-2.6.7-cy/init/main.c
@@ -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();

kernel/fork.c +0 -0

diff -rup linux-2.6.7/kernel/fork.c linux-2.6.7-cy/kernel/fork.c
@@ -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;
	/*

Signature · 2 lines

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

Russell King

2004-06-28 16:53 UTC #1 lore raw

Quoted 7 lines · 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.

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...

Signature · 4 lines

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

Coywolf Qi Hunt

author

2004-06-29 10:48 UTC #2 lore raw

Quoted 11 lines · Russell King

Quoted 7 lines · Coywolf Qi Hunt
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.

Quoted 8 lines

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;

Signature · 2 lines

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

Russell King

2004-06-29 10:58 UTC #3 lore raw

Quoted 5 lines · Coywolf Qi Hunt

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.

Quoted 2 lines

[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.

Signature · 4 lines

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

Coywolf Qi Hunt

author

2004-06-30 09:57 UTC #4 lore raw

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

Quoted 27 lines · Russell King

Quoted 5 lines · Coywolf Qi Hunt
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.
Quoted 4 lines
[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);

Quoted 7 lines

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.

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

arch/arm/mm/init.c +0 -0

diff -Nrup linux-2.6.7/arch/arm/mm/init.c linux-2.6.7-cy2/arch/arm/mm/init.c
@@ -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;
}

arch/arm26/mm/init.c +0 -0

diff -Nrup linux-2.6.7/arch/arm26/mm/init.c linux-2.6.7-cy2/arch/arm26/mm/init.c
@@ -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;

}

Signature · 2 lines

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

Chris Wedgwood

2004-06-29 11:12 UTC #5 lore raw

In reply to Russell King

Quoted 2 lines · Russell King

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

Coywolf Qi Hunt

author

2004-06-30 10:43 UTC #6 lore raw

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

In reply to Coywolf Qi Hunt

Quoted 5 lines · Coywolf Qi Hunt

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.

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

init/main.c +0 -0

diff -rup linux-2.6.7/init/main.c linux-2.6.7-cy/init/main.c
@@ -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();

kernel/fork.c +0 -0

diff -rup linux-2.6.7/kernel/fork.c linux-2.6.7-cy/kernel/fork.c
@@ -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;
	/*

Signature · 2 lines

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