Hi Udit, On Sun, 24 Sept 2023 at 22:10, Kumar, Udit <u-kum...@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-kum...@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-a...@ti.com> > >> Signed-off-by: Udit Kumar <u-kum...@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
