On 4/14/24 13:10, Kumar, Udit wrote:
Hello Patrice,
On 4/13/2024 1:54 PM, Patrice CHOTARD wrote:
On 4/12/24 17:53, Patrice Chotard wrote:
In case a new region is adjacent to a previous region with similar flag, this region is merged with its predecessor, but no check are done if this new added region is overlapping another region present in lmb (see reserved[3] which overlaps reserved[4]).
[..] phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align) { return lmb_alloc_base(lmb, size, align, LMB_ALLOC_ANYWHERE);
I think this series (v2) is not correct even if now the CI tests are OK. After re-reading carefully the lib_test_lmb_overlapping_reserve() test it appears to me there is a contradiction.
It's indicating that "check that calling lmb_reserve with overlapping regions fails"
but the very last test of lib_test_lmb_overlapping_reserve() has this comment : /* allocate 3rd region, coalesce with first and overlap with second */ and this test allows this overlap case.
It's not clear if LMB region can overlap each other or not ?
I would say partial overlap and coalescing with before one
May be Below can help
/* allocate 2nd region , This should coalesced all region into one
you will get one region as
Address --- Size
0x40010000 --- 0x30000
Next after this /* allocate 2nd region, which should be added as first region */
we will have two region like
Address --- Size
(0x40000000 -- 0x8000)
(0x40010000 --- 0x30000)
Now third request comes in
/* allocate 3rd region, coalesce with first and overlap with second */
which is address of 0x40008000 and size of 0x10000, Now this region to be added
is coalescing with first (0x40000000 -- 0x8000) and part of this overlap with (0x40010000 --- 0x30000).
So, what this patch does , merge all these into one region
as (0x40000000 -- 0x40000)
Hi Udit
Ok, but why the second test done in test/lib/lmb.c test should be considered as failed ?
ret = lmb_reserve(&lmb, 0x40010000, 0x10000); ut_asserteq(ret, 0); ASSERT_LMB(&lmb, ram, ram_size, 1, 0x40010000, 0x10000, 0, 0, 0, 0); /* allocate overlapping region should fail */ ret = lmb_reserve(&lmb, 0x40011000, 0x10000); ut_asserteq(ret, -1);
The 2 area 0x40010000 -- 0x10000 and 0x40011000 -- 0x10000 area overlaps each other and should be merged in one 0x40010000 -- 11000 ?
What is the criteria to merge or not 2 overlapping areas ?
For me overlapping shouldn't be authorized.
Patrice
Udit, your patch edb5824be17f ("lmb: remove overlapping region with next range") is authorizing LMB overlapping right ?
As said before this is checking overlap and coalescing and acting accordingly.
Patrice