Re: [PATCH] x86: seperate reserve_early and reserve_early_overlap_check

["H. Peter Anvin" <hpa@zytor.com>]

On 12/02/2009 06:31 PM, Yinghai Lu wrote:
>> 1. Renaming reserve_early() to reserve_early_overlap_check() is most
>> likely going to get overlooked, and people will use the "new"
>> reserve_early() thinking that they got the old one.
> kill reserve_early()
> and use
> reserve_early_overlap_check()
> reserve_early_overlap_not_check()

I would think that we should just leave the current reserve_early() in
place... it is what we want most of the time.

I guess I'd like to know specifically when one does *not* want an
overlap check, which is really the issue here.

>> 2. This creates overlapping ranges in the reservation array itself.
> 
> why?
> 
> if the range is retrieved from find_e820_area(). that range should be safe.
> we could add the early_res directly.

If there is no overlap, the current reserve_early() will work just fine.
 If there is overlap, then your code would create overlapping
reservations in the array, since it doesn't seem to check.

Either the new interface is unnecessary, or it is bad... I don't see any
other alternatives.  Unless the only point is to try to shave off a few
microseconds, but if so, it seems rather pointless to seek to the end of
the array first...

> 
> your patch seems to solve the multiple overlap ok problem.
> 
> these overlap check stuff is added by SGI guys to bandit their bios problem. maybe we can kill them at some point.
> 

It's probably a good idea to have them... errors happen.

One thing I would like to see is to change the underlying data structure
to instead of having (start, end, attributes) for each reservation, have
(start, attributes) for all address space.  Such a list would by
definition always be sorted, and overlaps would be trivial to detect.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.