Liang Ma <lian...@liangbit.com> 于2023年2月16日周四 22:04写道： > > 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. >
Thank you for the detailed explanation ! You may have already found the problem that this patch can only solve the fragmentation problem of 4k/16k/64k scenes, not work for 3k/15k/63k. For example, alloc 3k and align with 1k, also has this problem. Now this patch can't handle all similar scenarios，We already started working on a new soultion which aims to solve all similar problems . BTW, this patch is still useful, because even without fragmentation problems, the probability of finding a suitable elem from a larger size list is greater than from a smaller size list. > > >
