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.
> 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).
> 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.
-- Keir
> Thanks,
> Xiao
>
>>
>> On 09/11/2009 20:04, "Xiao Guangrong" <ericxiao.gr@xxxxxxxxx> wrote:
>>
>>> In clip_to_limit(), after memmove(&e820.map[i], &e820.map[i+1], ...), the
>>> original
>>> e820.map[i+1] become current e820.map[i] but the next loop count is i+1, so
>>> the original
>>> e820.map[i+1] will be skipped
>>>
>>> Actually, e820 is sorted form low to high by sanitize_e820_map(), so we can
>>> simply break
>>> the loop if we meet the item which overrun "limit"
>>>
>>> Signed-off-by: Xiao Guangrong <ericxiao.gr@xxxxxxxxx>
>>>
>>> diff -r 93bc06dd1161 -r 5e06f2790d93 xen/arch/x86/e820.c
>>> --- a/xen/arch/x86/e820.c Tue Nov 10 02:41:59 2009 +0800
>>> +++ b/xen/arch/x86/e820.c Tue Nov 10 03:51:08 2009 +0800
>>> @@ -389,6 +389,7 @@
>>> (e820.nr_map - i - 1) * sizeof(struct e820entry));
>>> e820.nr_map--;
>>> }
>>> + break;
>>> }
>>>
>>> if ( old_limit )
>>
>>
>>
>>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home