[PATCH] lmb: remove overlapping region with next range

[Mark Kettenis]

> Date: Mon, 25 Sep 2023 11:21:26 -0400
> From: Tom Rini <trini at konsulko.com>
> 
> On Sun, Sep 24, 2023 at 04:48:32PM +0530, Udit Kumar wrote:
> 
> > In case of new memory range to be added is coalesced
> > with any already added non last lmb region.
> > 
> > And there is possibility that, then region in which new memory
> > range added is not adjacent to next region. But have some
> > sections are overlapping.
> > 
> > So along with adjacency check with next lmb region,
> > check for overlap should be done.
> > 
> > In case overlap  is found, adjust and merge these two lmb
> > region into one.
> > 
> > Reported-by: Suman Anna <s-anna at ti.com>
> > Signed-off-by: Udit Kumar <u-kumar1 at ti.com>
> 
> This is good!  I wonder if this addresses the issue that has caused
> other platforms to need to set CONFIG_LMB_MAX_REGIONS=64 and I'm
> wondering if any of the other maintainers I've cc'd here can drop back
> to the default number of regions and then re-test their failure case?
> Thanks!

Hi Tom,

For the Apple M1 systems I don't think it makes sense to reduce the
number of regions.  These systems don't have TPL or SPL, have plenty
of memory (at least 8GB) and are really fast.  And we anticipate that
the number of memory regions will only grow in the future.

Cheers,

Mark

> > ---
> > logs
> > https://gist.github.com/uditkumarti/5d08c34442235ad270cfa863792ebcdc
> > seqeunce : line 1 to 13
> > without fix : line 96-100
> > with fix : line 199-202
> > 
> >  lib/lmb.c | 37 +++++++++++++++++++++++++++++++++----
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index b2c233edb6..2580d01d90 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -74,6 +74,16 @@ static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
> >  	return 0;
> >  }
> >  
> > +static long lmb_regions_overlap(struct lmb_region *rgn, unsigned long r1,
> > +				unsigned long r2)
> > +{
> > +	phys_addr_t base1 = rgn->region[r1].base;
> > +	phys_size_t size1 = rgn->region[r1].size;
> > +	phys_addr_t base2 = rgn->region[r2].base;
> > +	phys_size_t size2 = rgn->region[r2].size;
> > +
> > +	return lmb_addrs_overlap(base1, size1, base2, size2);
> > +}
> >  static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
> >  				 unsigned long r2)
> >  {
> > @@ -81,7 +91,6 @@ static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
> >  	phys_size_t size1 = rgn->region[r1].size;
> >  	phys_addr_t base2 = rgn->region[r2].base;
> >  	phys_size_t size2 = rgn->region[r2].size;
> > -
> >  	return lmb_addrs_adjacent(base1, size1, base2, size2);
> >  }
> >  
> > @@ -105,6 +114,23 @@ static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
> >  	lmb_remove_region(rgn, r2);
> >  }
> >  
> > +/*Assmptuon : base addr of region 1 < base addr of region 2*/
> > +static void lmb_fix_over_lap_regions(struct lmb_region *rgn, unsigned long r1,
> > +				     unsigned long r2)
> > +{
> > +	phys_addr_t base1 = rgn->region[r1].base;
> > +	phys_size_t size1 = rgn->region[r1].size;
> > +	phys_addr_t base2 = rgn->region[r2].base;
> > +	phys_size_t size2 = rgn->region[r2].size;
> > +
> > +	if (base1 + size1 > base2 + size2) {
> > +		printf("This will not be a case any time\n");
> > +		return;
> > +	}
> > +	rgn->region[r1].size = base2 + size2 - base1;
> > +	lmb_remove_region(rgn, r2);
> > +}
> > +
> >  void lmb_init(struct lmb *lmb)
> >  {
> >  #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> > @@ -249,7 +275,6 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
> >  		phys_size_t rgnflags = rgn->region[i].flags;
> >  		phys_addr_t end = base + size - 1;
> >  		phys_addr_t rgnend = rgnbase + rgnsize - 1;
> > -
> >  		if (rgnbase <= base && end <= rgnend) {
> >  			if (flags == rgnflags)
> >  				/* Already have this region, so we're done */
> > @@ -278,10 +303,14 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
> >  		}
> >  	}
> >  
> > -	if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
> > -		if (rgn->region[i].flags == rgn->region[i + 1].flags) {
> > +	if (i < rgn->cnt - 1 && rgn->region[i].flags == rgn->region[i + 1].flags)  {
> > +		if (lmb_regions_adjacent(rgn, i, i + 1)) {
> >  			lmb_coalesce_regions(rgn, i, i + 1);
> >  			coalesced++;
> > +		} else if (lmb_regions_overlap(rgn, i, i + 1)) {
> > +			/* fix overlapping area */
> > +			lmb_fix_over_lap_regions(rgn, i, i + 1);
> > +			coalesced++;
> >  		}
> >  	}
> >  
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Tom
> 
> [2:application/pgp-signature Show Save:signature.asc (659B)]
>