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 16:19:35 +0800
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Xiao Guangrong <ericxiao.gr@xxxxxxxxx>
Delivery-date: Tue, 10 Nov 2009 00:21:24 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C71ECA94.19BF5%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: <C71ECA94.19BF5%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird (Windows/20070728)

Keir Fraser wrote:
> On 10/11/2009 02:13, "Xiao Guangrong" <xiaoguangrong@xxxxxxxxxxxxxx> wrote:
>> Hi Keir,
>> Keir Fraser wrote:
>>> I think the 'break' is in the wrong place. Actually also I think the case of
>> Why we can't break the loop if we meet the "large" end address? what am i
>> missed?
> 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 :-)

>> Your patch work well, IMHO, double loop is inefficient
> Well, possibly. But really a typical e820 map will not have more than a
> small handful of offending RAM regions, hence there should be very few
> iterations of the outer loop. Also we already re-set the loop variable in
> the e820_change_range_type() case, so we effectively had the same double
> loop there already (and change_range_type will be by far the common case
> when we find a e820 region to clip).

Yeah, you are right, I missed it before :-)


>> I think we don't need reload loop hear, because e820_change_range_type() not
>> touch front object(it may merge with e820.map[i+1], but it not hurt us).
> It also does a full e820 merge operation at the end. I wouldn't really like
> to make assumptions about how much that modifies e820.

Xen-devel mailing list