Hi Dmitry, > alloc_sz = RTE_ALIGN_CEIL(RTE_ALIGN_CEIL(elt_size, align) + > MALLOC_ELEM_OVERHEAD, pg_sz); > I am submitting a patch regarding this
> 2. Alignment calculation depends on whether we allocated new pages or not: > > malloc_heap_alloc_on_heap_id(align = 0) -> > heap_alloc(align = 1) -> > find_suitable_element(align = RTE_CACHE_LINE_ROUNDUP(align)) > > malloc_heap_alloc_on_heap_id(align = 0) -> > alloc_more_mem_on_socket(align = 1) -> > try_expand_heap() -> ... -> > alloc_pages_on_heap(align = 1) -> > find_suitable_element(align = 1) I saw alloc_pages_on_heap() has a rollback if find_suitable_element() fails to find element. Now if we remove find_suitable_element(), don't we need to rollback, wouldn't it mean unnecessary allocation or is it handled somewhere that I didn't understand. On Sat, Jun 18, 2022 at 4:29 PM Dmitry Kozlyuk <dmitry.kozl...@gmail.com> wrote: > > Hi Fidaullah, > > Thanks for the fix, > Acked-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com> > > > Anatoly, I noticed a couple of other things while testing this. > > 1. Consider: > > elt_size = pg_sz - MALLOC_ELEM_OVERHEAD > rte_malloc(align=0) which is converted to align = 1. > > Obviously, such an element fits into one page, however: > > alloc_sz = RTE_ALIGN_CEIL(1 + pg_sz + > (MALLOC_ELEM_OVERHEAD - MALLOC_ELEM_OVERHEAD), > pg_sz) == 2 * pg_sz. > > This can unnecessarily hit an allocation limit from the system or EAL. > I suggest, in both places: > > alloc_sz = RTE_ALIGN_CEIL(RTE_ALIGN_CEIL(elt_size, align) + > MALLOC_ELEM_OVERHEAD, pg_sz); > > This would be symmetric with malloc_elem_can_hold(). > > 2. Alignment calculation depends on whether we allocated new pages or not: > > malloc_heap_alloc_on_heap_id(align = 0) -> > heap_alloc(align = 1) -> > find_suitable_element(align = RTE_CACHE_LINE_ROUNDUP(align)) > > malloc_heap_alloc_on_heap_id(align = 0) -> > alloc_more_mem_on_socket(align = 1) -> > try_expand_heap() -> ... -> > alloc_pages_on_heap(align = 1) -> > find_suitable_element(align = 1) > > Why do we call find_suitable_element() directly and not just return > and repeat the heap_alloc() attempt?
