2004-06-28 15:55:53

by Coywolf Qi Hunt

[permalink] [raw]
Subject: [BUG FIX] [PATCH] 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

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


2004-06-28 16:53:44

by Russell King

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

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

2004-06-29 10:48:46

by Coywolf Qi Hunt

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

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

2004-06-29 10:58:38

by Russell King

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

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

2004-06-29 11:12:15

by Chris Wedgwood

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

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

2004-06-30 09:57:56

by Coywolf Qi Hunt

[permalink] [raw]
Subject: [BUG FIX] [ARM/ARM26] find_memend_and_nodes bug fix

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

2004-06-30 10:43:42

by Coywolf Qi Hunt

[permalink] [raw]
Subject: [BUG FIX] fork_init() OOM bug on big highmem machine

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