[PATCH] lmb: remove overlapping region with next range

[Simon Glass]

Hi Udit,

On Sun, 24 Sept 2023 at 22:10, Kumar, Udit <u-kumar1 at ti.com> wrote:
>
> On 9/24/2023 7:21 PM, Heinrich Schuchardt wrote:
> >
> > Am 24. September 2023 13:18:32 MESZ schrieb Udit Kumar <u-kumar1 at ti.com>:
> >> 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>
> >> ---
> >> 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*/
> > Assumption
>
>
> Thanks Heinrich  will fix this typo in v2.
>
> Let me wait for other review feedback as well before proceeding for v2.
>
>
> > Regards Heinrich
> >
> >> +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++;
> >>              }
> >>      }
> >>

Can you please add a test to test/lib which fails before your change
and passes after?

Regards,
Simon