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?
> successful change_range_type() is also wrong, as i=0 will be skipped on the
> next iteration of the loop.
>
> Overall I decided that modifying the e820 map inside the iterator loop was
> just bad and confusing, so I've rewritten it in response to your bug
> discovery. Please take a look at xen-unstable:20419 and let me know if you
> see any issues.
Your patch work well, IMHO, double loop is inefficient, we can decrease the
loop counter if we need "memmove" it, like this:
if ( e820.map[i].addr < limit )
{
e820.map[i].size = limit - e820.map[i].addr;
}
else
{
memmove(&e820.map[i], &e820.map[i+1],
(e820.nr_map - i - 1) * sizeof(struct e820entry));
e820.nr_map--;
+ i--;
}
Also in the original code:
if ( e820_change_range_type(&e820, max(e820.map[i].addr, limit),
old_limit, E820_RAM, E820_UNUSABLE) )
{
/* Start again now e820 map must have changed. */
i = 0;
}
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).
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