On Wed, Feb 15, 2023 at 12:10:23PM +0100, Morten Brørup wrote: > +CC: Fidaullah Noonari <fidaullah.noon...@emumba.com>, your name also shows > up in the git log; perhaps you can help review this patch. > > > I gave up reviewing in depth, because the existing code is not easy to > quickly understand, and it would take too long for me. E.g. the > malloc_elem->size is field is undocumented, and find_suitable_element() calls > the malloc_elem_free_list_index() function with the raw size (as passed to > the function), but malloc_elem_free_list_insert() calls the > malloc_elem_free_list_index() with malloc_elem->size - MALLOC_ELEM_HEADER_LEN. > > Looking isolated at the patch itself... > > I agree with the way the patch modifies the ranges of the free list, and the > consequential removal of the "- 1" from the calculation of log2. > > Intuitively, the lists should cover ranges such as [0x100..0x3FF], which this > patch does, not [0x101..0x400], how it was previously... The ranges with this > patch make much more sense. > > So if the existing code is otherwise correct, i.e. handles the size > with/without MALLOC_ELEM_HEADER_LEN correctly, my gut feeling says this patch > is an improvement. > > Acked-by: Morten Brørup <m...@smartsharesystems.com> I run the test with DPDK malloc perf test. The issue is replicated. IMO, the whole process is if application use rte_malloc with a relative large alignment size. e.g. 4K alignment. Currently implementation will generate lots "fragment" because of the large alignment in related mem element free list. In the test code, 4K malloc size + 4k alignment will lead to the actually space is needed has to take from upper level mem element idx free list. The consequence is that will generate lots fragment element and those element is inserted to the current level mem free-list. However, when the rte_malloc select which free list to start scan with, it doesn't take the alignment into account, which ends up with waste some time in the current level free-list. If the scan logic take alignment into account, it might "smartly" skip current level and jump to the upper level directly.
>
