Skip to contentSkip to conversation

nr_pfns miscalculated

PATCH In review Scott Parish 4 participants 5 messages May 12, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nr_pfns miscalculated

If i'm not mistaken, this was what was intended.

sRp

Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
xen/arch/x86/mm.c +1 -3
@@ -209,9 +209,7 @@
         if ( e820.map[i].type == E820_RAM )
             continue;
         pfn = e820.map[i].addr >> PAGE_SHIFT;
-        nr_pfns = (e820.map[i].size +
-                   (e820.map[i].addr & ~PAGE_MASK) +
-                   ~PAGE_MASK) >> PAGE_SHIFT;
+        nr_pfns = (e820.map[i].size + ~PAGE_MASK) >> PAGE_SHIFT;
         for ( j = 0; j < nr_pfns; j++ )
         {
             if ( !pfn_valid(pfn+j) )

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw
Quoted 1 line · Scott Parish

If i'm not mistaken, this was what was intended.

Looks to me like the current code tries to handle the case where an e820 entry starts not on a page boundary -- not sure this is needed...

christian

#2 lore raw
Quoted 2 lines · Christian Limpach

Looks to me like the current code tries to handle the case where an e820 entry starts not on a page boundary -- not sure this is needed...

It's probably rounding the wrong way: should consider only pages completely covered by the e820 range.

-- Keir

#3 lore raw
Quoted 8 lines · Keir Fraser
Quoted 2 lines · Christian Limpach

Looks to me like the current code tries to handle the case where an e820 entry starts not on a page boundary -- not sure this is needed...

It's probably rounding the wrong way: should consider only pages completely covered by the e820 range.

At least the current code is consistent.

christian

#4 lore raw
Quoted 7 lines · Christian Limpach
Quoted 6 lines · Keir Fraser
Quoted 2 lines · Christian Limpach

Looks to me like the current code tries to handle the case where an e820 entry starts not on a page boundary -- not sure this is needed...

Quoted 6 lines
Quoted 2 lines

It's probably rounding the wrong way: should consider only pages completely covered by the e820 range.

At least the current code is consistent.

christian

Actually it's deciding what should be marked as 'I/O memory'. The current code includes fractional pages -- since such pages are never registered with the RAM allocator, this policy doesn't seem harmful.

-- Keir