Hi Sughosh,
On 2/13/25 2:11 PM, 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 an assumption that
the overlap would not encompass the existing region, and in such a
scenario, it prints a message and returns without making the
fix. Handle the case of an encompassing overlap also in the function.
Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
Reported-by: Quentin Schulz <quentin.sch...@cherry.de>
---
Note: To be applied after an A-b/R-b/T-b from the original author
of the lmb_fix_over_lap_regions() function on this
lib/lmb.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index aeaf120f57d..a5216bdccc7 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -111,11 +111,9 @@ static void lmb_fix_over_lap_regions(struct alist
*lmb_rgn_lst,
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;
+ if (base1 + size1 < base2 + size2)
+ rgn[r1].size = base2 + size2 - base1;
+
lmb_remove_region(lmb_rgn_lst, r2);
Mm I have a feeling this function was improperly named, this is
essentially just merging two regions that are overlapping. Similarly to
lmb_coalesce_regions except it handles overlaps too.
I'm wondering if we shouldn't simply merge lmb_regions_adjacent and
lmb_regions_overlap, and lmb_coalesce_regions and
lmb_fix_over_lap_regions into one function each? e.g.
lmb_regions_mergeable() and lmb_merge_regions()?
For what it's worth, I was able to trigger this printf with a pxe load
at an address 0xb0b and have the kernel_addr_r at 0xb00 for example.
What I don't understand is why
1) aside from the printf, it still booted seemingly properly
2) why interrupting the transfer and starting it again would not make
those messages appear anymore
Logically though, this patch seems to be ok if the purpose is to merge
two regions.
Thanks for looking into this,
Quentin
