[dpdk-dev] [PATCH v6 03/21] malloc: index heaps using heap ID rather than NUMA node

[Alejandro Lucero]

On Thu, Sep 27, 2018 at 2:18 PM Burakov, Anatoly <anatoly.burakov at intel.com>
wrote:

> On 27-Sep-18 2:01 PM, Alejandro Lucero wrote:
> > On Thu, Sep 27, 2018 at 11:47 AM Anatoly Burakov <
> anatoly.burakov at intel.com>
> > wrote:
> >
> >> Switch over all parts of EAL to use heap ID instead of NUMA node
> >> ID to identify heaps. Heap ID for DPDK-internal heaps is NUMA
> >> node's index within the detected NUMA node list. Heap ID for
> >> external heaps will be order of their creation.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> >> ---
> >>   config/common_base                            |  1 +
> >>   config/rte_config.h                           |  1 +
> >>   .../common/include/rte_eal_memconfig.h        |  4 +-
> >>   .../common/include/rte_malloc_heap.h          |  1 +
> >>   lib/librte_eal/common/malloc_heap.c           | 98 +++++++++++++------
> >>   lib/librte_eal/common/malloc_heap.h           |  3 +
> >>   lib/librte_eal/common/rte_malloc.c            | 41 +++++---
> >>   7 files changed, 106 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/config/common_base b/config/common_base
> >> index 155c7d40e..b52770b27 100644
> >> --- a/config/common_base
> >> +++ b/config/common_base
> >> @@ -61,6 +61,7 @@ CONFIG_RTE_CACHE_LINE_SIZE=64
> >>   CONFIG_RTE_LIBRTE_EAL=y
> >>   CONFIG_RTE_MAX_LCORE=128
> >>   CONFIG_RTE_MAX_NUMA_NODES=8
> >> +CONFIG_RTE_MAX_HEAPS=32
> >>   CONFIG_RTE_MAX_MEMSEG_LISTS=64
> >>   # each memseg list will be limited to either RTE_MAX_MEMSEG_PER_LIST
> pages
> >>   # or RTE_MAX_MEM_MB_PER_LIST megabytes worth of memory, whichever is
> >> smaller
> >> diff --git a/config/rte_config.h b/config/rte_config.h
> >> index 567051b9c..5dd2ac1ad 100644
> >> --- a/config/rte_config.h
> >> +++ b/config/rte_config.h
> >> @@ -24,6 +24,7 @@
> >>   #define RTE_BUILD_SHARED_LIB
> >>
> >>   /* EAL defines */
> >> +#define RTE_MAX_HEAPS 32
> >>   #define RTE_MAX_MEMSEG_LISTS 128
> >>   #define RTE_MAX_MEMSEG_PER_LIST 8192
> >>   #define RTE_MAX_MEM_MB_PER_LIST 32768
> >> diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h
> >> b/lib/librte_eal/common/include/rte_eal_memconfig.h
> >> index 6baa6854f..d7920a4e0 100644
> >> --- a/lib/librte_eal/common/include/rte_eal_memconfig.h
> >> +++ b/lib/librte_eal/common/include/rte_eal_memconfig.h
> >> @@ -72,8 +72,8 @@ struct rte_mem_config {
> >>
> >>          struct rte_tailq_head tailq_head[RTE_MAX_TAILQ]; /**< Tailqs
> for
> >> objects */
> >>
> >> -       /* Heaps of Malloc per socket */
> >> -       struct malloc_heap malloc_heaps[RTE_MAX_NUMA_NODES];
> >> +       /* Heaps of Malloc */
> >> +       struct malloc_heap malloc_heaps[RTE_MAX_HEAPS];
> >>
> >>          /* address of mem_config in primary process. used to map shared
> >> config into
> >>           * exact same address the primary process maps it.
> >> diff --git a/lib/librte_eal/common/include/rte_malloc_heap.h
> >> b/lib/librte_eal/common/include/rte_malloc_heap.h
> >> index d43fa9097..e7ac32d42 100644
> >> --- a/lib/librte_eal/common/include/rte_malloc_heap.h
> >> +++ b/lib/librte_eal/common/include/rte_malloc_heap.h
> >> @@ -27,6 +27,7 @@ struct malloc_heap {
> >>
> >>          unsigned alloc_count;
> >>          size_t total_size;
> >> +       unsigned int socket_id;
> >>   } __rte_cache_aligned;
> >>
> >>   #endif /* _RTE_MALLOC_HEAP_H_ */
> >> diff --git a/lib/librte_eal/common/malloc_heap.c
> >> b/lib/librte_eal/common/malloc_heap.c
> >> index 3c8e2063b..1d1e35708 100644
> >> --- a/lib/librte_eal/common/malloc_heap.c
> >> +++ b/lib/librte_eal/common/malloc_heap.c
> >> @@ -66,6 +66,21 @@ check_hugepage_sz(unsigned flags, uint64_t
> hugepage_sz)
> >>          return check_flag & flags;
> >>   }
> >>
> >> +int
> >> +malloc_socket_to_heap_id(unsigned int socket_id)
> >> +{
> >> +       struct rte_mem_config *mcfg =
> >> rte_eal_get_configuration()->mem_config;
> >> +       int i;
> >> +
> >> +       for (i = 0; i < RTE_MAX_HEAPS; i++) {
> >> +               struct malloc_heap *heap = &mcfg->malloc_heaps[i];
> >> +
> >> +               if (heap->socket_id == socket_id)
> >> +                       return i;
> >> +       }
> >> +       return -1;
> >> +}
> >> +
> >>   /*
> >>    * Expand the heap with a memory area.
> >>    */
> >> @@ -93,12 +108,13 @@ malloc_add_seg(const struct rte_memseg_list *msl,
> >>          struct rte_mem_config *mcfg =
> >> rte_eal_get_configuration()->mem_config;
> >>          struct rte_memseg_list *found_msl;
> >>          struct malloc_heap *heap;
> >> -       int msl_idx;
> >> +       int msl_idx, heap_idx;
> >>
> >>          if (msl->external)
> >>                  return 0;
> >>
> >> -       heap = &mcfg->malloc_heaps[msl->socket_id];
> >> +       heap_idx = malloc_socket_to_heap_id(msl->socket_id);
> >>
> >
> > malloc_socket_to_heap_id can return -1 so it requires to handle that
> > possibility.
> >
>
> Not really, this is called from memseg walk function - we know the msl
> and its socket ID are valid. Or at least something has gone *very* wrong
> if we got a -1 result :) However, i guess this check won't hurt.
>
>
Although that error is impossible now, not doing the check could be a
problem if there is another code path in the future where socket_id has not
checked yet.


> --
> Thanks,
> Anatoly
>