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-a...@ti.com>
> Signed-off-by: Udit Kumar <u-kum...@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!

> ---
> 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

Attachment: signature.asc
Description: PGP signature

Reply via email to