[PATCH tip:x86/urgent] x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

[PATCH tip:x86/urgent] x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

[Tejun Heo]

From: Yinghai Lu <yinghai@kernel.org>

numa_cleanup_meminfo() trims each memblk between low (0) and high
(max_pfn) limits and discards empty ones.  However, the emptiness
detection incorrectly used equality test.  If the start of a memblk is
higher than max_pfn, it is empty but fails the equality test and
doesn't get discarded.

Fix it by using >= instead of ==.

tj: Extracted the actual fix from the original patch and rewrote patch
    description.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
Ingo, can you please push this to mainline?  Once it hits x86/urgent,
I'll regenerate the 32bit NUMA unification branch on top of it with
Yinghai's other patches and send pull request for x86/mm.

Thank you.

 arch/x86/mm/numa_64.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/arch/x86/mm/numa_64.c
===================================================================
--- work.orig/arch/x86/mm/numa_64.c
+++ work/arch/x86/mm/numa_64.c
@@ -191,7 +191,7 @@ int __init numa_cleanup_meminfo(struct n
 		bi->end = min(bi->end, high);
 
 		/* and there's no empty block */
-		if (bi->start == bi->end) {
+		if (bi->start >= bi->end) {
 			numa_remove_memblk_from(i--, mi);
 			continue;
 		}

Re: [PATCH tip:x86/urgent] x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

[Ingo Molnar]

* Tejun Heo <tj@kernel.org> wrote:

> From: Yinghai Lu <yinghai@kernel.org>
> 
> numa_cleanup_meminfo() trims each memblk between low (0) and high
> (max_pfn) limits and discards empty ones.  However, the emptiness
> detection incorrectly used equality test.  If the start of a memblk is
> higher than max_pfn, it is empty but fails the equality test and
> doesn't get discarded.
> 
> Fix it by using >= instead of ==.
> 
> tj: Extracted the actual fix from the original patch and rewrote patch
>     description.

Well, the bit still missing from the description is why this fix matters for 
x86/urgent: under what scenarios does this cause problems, how frequent is it, 
how severe are the prolems, etc. etc. Would be nice to have something about 
that in the changelog as well.

Thanks,

	Ingo

[PATCH REPOST tip:x86/urgent] x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

[Tejun Heo]

From: Yinghai Lu <yinghai@kernel.org>

numa_cleanup_meminfo() trims each memblk between low (0) and high
(max_pfn) limits and discards empty ones.  However, the emptiness
detection incorrectly used equality test.  If the start of a memblk is
higher than max_pfn, it is empty but fails the equality test and
doesn't get discarded.

The condition triggers when max_pfn is lower than start of a NUMA node
and results in memory misconfiguration - leading to WARN_ON()s and
other funnies.  The bug was discovered in devel branch where 32bit too
uses this code path for NUMA init.  If a node is above the addressing
limit, max_pfn ends up lower than the node triggering this problem.

The failure hasn't been observed on x86-64 but is still possible with
broken hardware e820/NUMA info.  As the fix is very low risk, it would
be better to apply it even for 64bit.

Fix it by using >= instead of ==.

tj: Extracted the actual fix from the original patch and rewrote patch
    description.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
Here's the patch with updated description.  Thank you.

 arch/x86/mm/numa_64.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/arch/x86/mm/numa_64.c
===================================================================
--- work.orig/arch/x86/mm/numa_64.c
+++ work/arch/x86/mm/numa_64.c
@@ -191,7 +191,7 @@ int __init numa_cleanup_meminfo(struct n
 		bi->end = min(bi->end, high);
 
 		/* and there's no empty block */
-		if (bi->start == bi->end) {
+		if (bi->start >= bi->end) {
 			numa_remove_memblk_from(i--, mi);
 			continue;
 		}

[tip:x86/urgent] x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

[tip-bot for Yinghai Lu]

Commit-ID:  2be19102b71c1a45d37fec50303791daa1a06869
Gitweb:     http://git.kernel.org/tip/2be19102b71c1a45d37fec50303791daa1a06869
Author:     Yinghai Lu <yinghai@kernel.org>
AuthorDate: Sun, 1 May 2011 19:12:04 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 1 May 2011 19:15:11 +0200

x86, NUMA: Fix empty memblk detection in numa_cleanup_meminfo()

numa_cleanup_meminfo() trims each memblk between low (0) and
high (max_pfn) limits and discards empty ones.  However, the
emptiness detection incorrectly used equality test.  If the
start of a memblk is higher than max_pfn, it is empty but fails
the equality test and doesn't get discarded.

The condition triggers when max_pfn is lower than start of a
NUMA node and results in memory misconfiguration - leading to
WARN_ON()s and other funnies.  The bug was discovered in devel
branch where 32bit too uses this code path for NUMA init.  If a
node is above the addressing limit, max_pfn ends up lower than
the node triggering this problem.

The failure hasn't been observed on x86-64 but is still possible
with broken hardware e820/NUMA info.  As the fix is very low
risk, it would be better to apply it even for 64bit.

Fix it by using >= instead of ==.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
[ Extracted the actual fix from the original patch and rewrote patch description. ]
Signed-off-by: Tejun Heo <tj@kernel.org>
Link: http://lkml.kernel.org/r/20110501171204.GO29280@htj.dyndns.org
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/mm/numa_64.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/mm/numa_64.c b/arch/x86/mm/numa_64.c
index e8c00cc..85b52fc 100644
--- a/arch/x86/mm/numa_64.c
+++ b/arch/x86/mm/numa_64.c
@@ -306,7 +306,7 @@ int __init numa_cleanup_meminfo(struct numa_meminfo *mi)
 		bi->end = min(bi->end, high);
 
 		/* and there's no empty block */
-		if (bi->start == bi->end) {
+		if (bi->start >= bi->end) {
 			numa_remove_memblk_from(i--, mi);
 			continue;
 		}