| Message ID | 20250220095654.121634-3-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | lmb: miscellaneous fixes and improvements | expand |
On 20.02.25 10:56, Sughosh Ganu wrote: > The lmb_fix_over_lap_regions() function is called if the added region > overlaps with an existing region. The function then fixes the overlap > and removes the redundant region. However, it makes certain > assumptions. One assumption is that the overlap would not encompass > the existing region. Another assumption is that the overlap only > occurs between two regions -- the scenario of the added region > overlapping multiple existing regions is not being handled. Handle > these cases by instead calling lmb_resize_regions(). Also remove the > now superfluous lmb_fix_over_lap_regions(). > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V1: > * Use lmb_resize_regions() to fix the overlap instead of > lmb_fix_over_lap_regions(). > * Remove the now superfluous lmb_fix_over_lap_regions(). > > > Note: To be applied after an A-b/R-b/T-b from the original author of > lmb_fix_over_lap_regions(). > > > lib/lmb.c | 30 +++++++++--------------------- > 1 file changed, 9 insertions(+), 21 deletions(-) > > diff --git a/lib/lmb.c b/lib/lmb.c > index d7d2c8c6dfd..207f059ceb9 100644 > --- a/lib/lmb.c > +++ b/lib/lmb.c > @@ -100,25 +100,6 @@ static void lmb_coalesce_regions(struct alist *lmb_rgn_lst, unsigned long r1, > lmb_remove_region(lmb_rgn_lst, r2); > } > > -/*Assumption : base addr of region 1 < base addr of region 2*/ > -static void lmb_fix_over_lap_regions(struct alist *lmb_rgn_lst, > - unsigned long r1, unsigned long r2) > -{ > - struct lmb_region *rgn = lmb_rgn_lst->data; > - > - phys_addr_t base1 = rgn[r1].base; > - phys_size_t size1 = rgn[r1].size; > - phys_addr_t base2 = rgn[r2].base; > - phys_size_t size2 = rgn[r2].size; > - > - if (base1 + size1 > base2 + size2) { > - printf("This will not be a case any time\n"); > - return; > - } > - rgn[r1].size = base2 + size2 - base1; > - lmb_remove_region(lmb_rgn_lst, r2); > -} > - > static long lmb_resize_regions(struct alist *lmb_rgn_lst, > unsigned long idx_start, > phys_addr_t base, phys_size_t size) > @@ -239,8 +220,15 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base, > lmb_coalesce_regions(lmb_rgn_lst, i, i + 1); > coalesced++; > } else if (lmb_regions_overlap(lmb_rgn_lst, i, i + 1)) { > - /* fix overlapping area */ > - lmb_fix_over_lap_regions(lmb_rgn_lst, i, i + 1); > + /* fix overlapping areas */ > + phys_addr_t rgnbase = rgn[i].base; > + phys_size_t rgnsize = rgn[i].size; > + > + ret = lmb_resize_regions(lmb_rgn_lst, i, > + rgnbase, rgnsize); > + if (ret < 0) > + return -1; > + > coalesced++; > } > } It would be great if in a future patch you could document all LMB functions like lmb_resize_regions() which lack a function description. Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
diff --git a/lib/lmb.c b/lib/lmb.c index d7d2c8c6dfd..207f059ceb9 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -100,25 +100,6 @@ static void lmb_coalesce_regions(struct alist *lmb_rgn_lst, unsigned long r1, lmb_remove_region(lmb_rgn_lst, r2); } -/*Assumption : base addr of region 1 < base addr of region 2*/ -static void lmb_fix_over_lap_regions(struct alist *lmb_rgn_lst, - unsigned long r1, unsigned long r2) -{ - struct lmb_region *rgn = lmb_rgn_lst->data; - - phys_addr_t base1 = rgn[r1].base; - phys_size_t size1 = rgn[r1].size; - phys_addr_t base2 = rgn[r2].base; - phys_size_t size2 = rgn[r2].size; - - if (base1 + size1 > base2 + size2) { - printf("This will not be a case any time\n"); - return; - } - rgn[r1].size = base2 + size2 - base1; - lmb_remove_region(lmb_rgn_lst, r2); -} - static long lmb_resize_regions(struct alist *lmb_rgn_lst, unsigned long idx_start, phys_addr_t base, phys_size_t size) @@ -239,8 +220,15 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base, lmb_coalesce_regions(lmb_rgn_lst, i, i + 1); coalesced++; } else if (lmb_regions_overlap(lmb_rgn_lst, i, i + 1)) { - /* fix overlapping area */ - lmb_fix_over_lap_regions(lmb_rgn_lst, i, i + 1); + /* fix overlapping areas */ + phys_addr_t rgnbase = rgn[i].base; + phys_size_t rgnsize = rgn[i].size; + + ret = lmb_resize_regions(lmb_rgn_lst, i, + rgnbase, rgnsize); + if (ret < 0) + return -1; + coalesced++; } }
The lmb_fix_over_lap_regions() function is called if the added region overlaps with an existing region. The function then fixes the overlap and removes the redundant region. However, it makes certain assumptions. One assumption is that the overlap would not encompass the existing region. Another assumption is that the overlap only occurs between two regions -- the scenario of the added region overlapping multiple existing regions is not being handled. Handle these cases by instead calling lmb_resize_regions(). Also remove the now superfluous lmb_fix_over_lap_regions(). Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V1: * Use lmb_resize_regions() to fix the overlap instead of lmb_fix_over_lap_regions(). * Remove the now superfluous lmb_fix_over_lap_regions(). Note: To be applied after an A-b/R-b/T-b from the original author of lmb_fix_over_lap_regions(). lib/lmb.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-)