On Tue, 2007-02-13 at 18:05 +1100, Michael Neuling wrote:
> > At present calling lmb_reserve() (and hence lmb_add_region()) twice
> > for exactly the same memory region will cause strange behaviour.
> > 
> > This makes life difficult when booting from a flat device tree with
> > memory reserve map.  Which regions are automatically reserved by the
> > kernel has changed over time, so it's quite possible a newer kernel
> > could attempt to auto-reserve a region which is also explicitly listed
> > in the device tree's reserve map, leading to trouble.
> > 
> > This patch avoids the problem by making lmb_reserve() ignore a call to
> > reserve a previously reserved region.  It also removes a now redundant
> > test designed to avoid one specific case of the problem noted above.
> > 
> > At present, this patch deals only with duplicate reservations of an
> > identical region.  Attempting to reserve two different, but
> > overlapping regions will still cause problems.  I might post another
> > patch later dealing with this case, but I'm avoiding it now since it
> > is substantially more complicated to deal with, less likely to occur
> > and more likely to indicate a genuine bug elsewhere if it does occur.
> > 
> > Signed-off-by: David Gibson <dwg@au1.ibm.com>
> > ---
> > 
> > 
> >  arch/powerpc/kernel/prom.c |    3 ---
> >  arch/powerpc/mm/lmb.c      |    4 ++++
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > Index: working-2.6/arch/powerpc/mm/lmb.c
> > ===================================================================
> > --- working-2.6.orig/arch/powerpc/mm/lmb.c	2007-02-06 16:21:02.000000000 +
> 1100
> > +++ working-2.6/arch/powerpc/mm/lmb.c	2007-02-06 16:22:32.000000000 +1100
> > @@ -146,6 +146,10 @@ static long __init lmb_add_region(struct
> >  		unsigned long rgnbase = rgn->region[i].base;
> >  		unsigned long rgnsize = rgn->region[i].size;
> >  
> > +		if ((rgnbase == base) && (rgnsize == size))
> > +			/* Already have this region, so we're done */
> > +			return 0;
> 
> This might indicate that two things actually want to use the same memory
> region.  How about we print a warning?

It'd be nice, but it'll spit out lots of spurious warnings for exactly
the reason David describes - things that used to be in the reserve map
are now auto reserved.

What we probably should warn for is overlapping, but not duplicated
regions - I can't think if any reason that _doesn't_ indicate a bug.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person