Re: [PATCH] x86: seperate reserve_early and reserve_early_overlap_check

[Yinghai Lu <yinghai@kernel.org>]

H. Peter Anvin wrote:
> On 11/25/2009 12:58 AM, Yinghai Lu wrote:
>> when the area is from find_e820_area(), it could be overlapped with others.
>>
>> so just add it directly. the new reserve_early()
>>
>> and rename old reserve_early() to reserve_early_overlap_check()
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> Hi Yinghai,
> 
> I had this patch in my queue but it looks like I had overlooked it as it
> arrived during the U.S. holiday; I apologize profusely!
> 
> I'm concerned about several things with this patch, which doesn't mean
> it isn't fulfilling a genuine need:
> 
> 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()
?
> 
> 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.

> 
> What it looks to me is what we need is actually a
> reserve_early_clobber() which does what the current reserve_early() does
> except that it ignores the overlap_ok flag on existing reservations.
> 
> I have attached an untested patch to do that.  Note that I don't have
> any callers for reserve_early_clobber(), since one effect of changing
> the semantics of an existing function in the way you did is that the
> patch contains the call sites that *didn't* need modification rather
> than the one that *did* need modification.  The call sites that want the
> new semantics need to be modified.  As such, it's possible that the
> comment I added is completely wrong, I really need some further
> information on this.
> 
> [Note: the function __reserve_early() hasn't actually changed; I just
> moved it ahead of drop_overlaps().]
> 

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.

YH