This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH] e820: fix clip_to_limit()

To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] e820: fix clip_to_limit()
From: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxx>
Date: Tue, 10 Nov 2009 17:18:44 +0800
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Xiao Guangrong <ericxiao.gr@xxxxxxxxx>
Delivery-date: Tue, 10 Nov 2009 01:20:32 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C71ED74B.19C04%keir.fraser@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C71ED74B.19C04%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (Windows/20070728)

Keir Fraser wrote:
> On 10/11/2009 08:19, "Xiao Guangrong" <xiaoguangrong@xxxxxxxxxxxxxx> wrote:
>>> Firstly, your 'break' was not inside that if-else block; it was right at the
>>> end of the for loop. Secondly, just because we found one RAM region entirely
>>> beyond the end of the clip boundary, does not mean there isn't another. We
>>> can't just bail -- we have to iterate all the way to the end of the e820
>>> map.
>> I think that sanitize_e820_map() can sort e820 items from low address
>> to high address, so, if we meet one e820 item beyond the end of the clip
>> boundary, subsequent items also beyond it.
>> Maybe I misunderstand sanitize_e820_map()? I'll reread it :-)
> No, you understand it. And if we meet one e820 item beyond the end of the
> clip boundary, all subsequent items are also beyond it. But that doesn't
> mean we shouldn't handle them -- in fact we must handle them, as one of them
> could be E820_RAM. Right?

Yeah, It's my mistake, Thanks very much, Keir!

And I think find_max_pfn() can be optimized. like this:

--- ../a/xen/arch/x86/e820.c    2009-08-06 21:57:27.000000000 +0800
+++ ../b/xen/arch/x86/e820.c    2009-10-25 17:31:42.762997342 +0800
@@ -312,8 +312,9 @@ static unsigned long __init find_max_pfn

-    for (i = 0; i < e820.nr_map; i++) {
+    for (i = e820.nr_map -1; i >= 0; i--) {
         unsigned long start, end;
         /* RAM? */
         if (e820.map[i].type != E820_RAM)
@@ -321,8 +322,8 @@ static unsigned long __init find_max_pfn
         end = PFN_DOWN(e820.map[i].addr + e820.map[i].size);
         if (start >= end)
-        if (end > max_pfn)
-            max_pfn = end;
+       max_pfn = end;
+       break;

     return max_pfn;


Xen-devel mailing list