Re: [dpdk-dev] [PATCH v9 1/2] mem: balanced allocation of hugepages

[Ilya Maximets <i.maximets@samsung.com>]

On 29.06.2017 08:48, Ilya Maximets wrote:
> On 29.06.2017 08:32, Hemant Agrawal wrote:
>> On 6/27/2017 3:54 PM, Ilya Maximets wrote:
>>> Currently EAL allocates hugepages one by one not paying attention
>>> from which NUMA node allocation was done.
>>>
>>> Such behaviour leads to allocation failure if number of available
>>> hugepages for application limited by cgroups or hugetlbfs and
>>> memory requested not only from the first socket.
>>>
>>> Example:
>>>     # 90 x 1GB hugepages availavle in a system
>>>
>>>     cgcreate -g hugetlb:/test
>>>     # Limit to 32GB of hugepages
>>>     cgset -r hugetlb.1GB.limit_in_bytes=34359738368 test
>>>     # Request 4GB from each of 2 sockets
>>>     cgexec -g hugetlb:test testpmd --socket-mem=4096,4096 ...
>>>
>>>     EAL: SIGBUS: Cannot mmap more hugepages of size 1024 MB
>>>     EAL: 32 not 90 hugepages of size 1024 MB allocated
>>>     EAL: Not enough memory available on socket 1!
>>>          Requested: 4096MB, available: 0MB
>>>     PANIC in rte_eal_init():
>>>     Cannot init memory
>>>
>>>     This happens beacause all allocated pages are
>>>     on socket 0.
>>>
>>> Fix this issue by setting mempolicy MPOL_PREFERRED for each hugepage
>>> to one of requested nodes using following schema:
>>>
>>>     1) Allocate essential hugepages:
>>>         1.1) Allocate as many hugepages from numa N to
>>>              only fit requested memory for this numa.
>>>         1.2) repeat 1.1 for all numa nodes.
>>>     2) Try to map all remaining free hugepages in a round-robin
>>>        fashion.
>>>     3) Sort pages and choose the most suitable.
>>>
>>> In this case all essential memory will be allocated and all remaining
>>> pages will be fairly distributed between all requested nodes.
>>>
>>> New config option RTE_EAL_NUMA_AWARE_HUGEPAGES introduced and
>>> enabled by default for linuxapp except armv7 and dpaa2.
>>> Enabling of this option adds libnuma as a dependency for EAL.
>>>
>>> Fixes: 77988fc08dc5 ("mem: fix allocating all free hugepages")
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>> ---
>>>  config/common_base                        |   1 +
>>>  config/common_linuxapp                    |   1 +
>>>  config/defconfig_arm-armv7a-linuxapp-gcc  |   3 +
>>>  config/defconfig_arm64-dpaa2-linuxapp-gcc |   3 +
>>>  lib/librte_eal/linuxapp/eal/Makefile      |   3 +
>>>  lib/librte_eal/linuxapp/eal/eal_memory.c  | 120 ++++++++++++++++++++++++++++--
>>>  mk/rte.app.mk                             |   3 +
>>>  7 files changed, 126 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/config/common_base b/config/common_base
>>> index f6aafd1..660588a 100644
>>> --- a/config/common_base
>>> +++ b/config/common_base
>>> @@ -103,6 +103,7 @@ CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
>>>  CONFIG_RTE_EAL_IGB_UIO=n
>>>  CONFIG_RTE_EAL_VFIO=n
>>>  CONFIG_RTE_MALLOC_DEBUG=n
>>> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
>>>
>>>  #
>>>  # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing.
>>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>>> index b3cf41b..64bef87 100644
>>> --- a/config/common_linuxapp
>>> +++ b/config/common_linuxapp
>>> @@ -35,6 +35,7 @@
>>>  CONFIG_RTE_EXEC_ENV="linuxapp"
>>>  CONFIG_RTE_EXEC_ENV_LINUXAPP=y
>>>
>>> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=y
>>>  CONFIG_RTE_EAL_IGB_UIO=y
>>>  CONFIG_RTE_EAL_VFIO=y
>>>  CONFIG_RTE_KNI_KMOD=y
>>> diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
>>> index 19607eb..e06b1d4 100644
>>> --- a/config/defconfig_arm-armv7a-linuxapp-gcc
>>> +++ b/config/defconfig_arm-armv7a-linuxapp-gcc
>>> @@ -47,6 +47,9 @@ CONFIG_RTE_ARCH_STRICT_ALIGN=y
>>>  CONFIG_RTE_TOOLCHAIN="gcc"
>>>  CONFIG_RTE_TOOLCHAIN_GCC=y
>>>
>>> +# NUMA is not supported on ARM
>>> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
>>> +
>>>  # ARM doesn't have support for vmware TSC map
>>>  CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
>>>
>>> diff --git a/config/defconfig_arm64-dpaa2-linuxapp-gcc b/config/defconfig_arm64-dpaa2-linuxapp-gcc
>>> index 2304ab6..f78449d 100644
>>> --- a/config/defconfig_arm64-dpaa2-linuxapp-gcc
>>> +++ b/config/defconfig_arm64-dpaa2-linuxapp-gcc
>>> @@ -45,6 +45,9 @@ CONFIG_RTE_CACHE_LINE_SIZE=64
>>>
>>>  CONFIG_RTE_PKTMBUF_HEADROOM=256
>>>
>>> +# Doesn't support NUMA
>>> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=y
>>> +
>>
>> DPAA2 does not support NUMA so,
>> CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> 
> Oh, sorry. Just typo.
> Thanks for catching this.

Fixed. Hemant, please, check the new version (v10).

> Sergio, I'll send v10 with only this change and will keep your
> acked-by because the change is trivial.
> 
>>>  #
>>>  # Compile Support Libraries for DPAA2
>>>  #
>>> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
>>> index 640afd0..8651e27 100644
>>> --- a/lib/librte_eal/linuxapp/eal/Makefile
>>> +++ b/lib/librte_eal/linuxapp/eal/Makefile
>>> @@ -50,6 +50,9 @@ LDLIBS += -ldl
>>>  LDLIBS += -lpthread
>>>  LDLIBS += -lgcc_s
>>>  LDLIBS += -lrt
>>> +ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y)
>>> +LDLIBS += -lnuma
>>> +endif
>>>
>>>  # specific to linuxapp exec-env
>>>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> index e17c9cb..647d89c 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> @@ -54,6 +54,10 @@
>>>  #include <sys/time.h>
>>>  #include <signal.h>
>>>  #include <setjmp.h>
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +#include <numa.h>
>>> +#include <numaif.h>
>>> +#endif
>>>
>>>  #include <rte_log.h>
>>>  #include <rte_memory.h>
>>> @@ -348,6 +352,14 @@ static int huge_wrap_sigsetjmp(void)
>>>      return sigsetjmp(huge_jmpenv, 1);
>>>  }
>>>
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +/* Callback for numa library. */
>>> +void numa_error(char *where)
>>> +{
>>> +    RTE_LOG(ERR, EAL, "%s failed: %s\n", where, strerror(errno));
>>> +}
>>> +#endif
>>> +
>>>  /*
>>>   * Mmap all hugepages of hugepage table: it first open a file in
>>>   * hugetlbfs, then mmap() hugepage_sz data in it. If orig is set, the
>>> @@ -356,18 +368,78 @@ static int huge_wrap_sigsetjmp(void)
>>>   * map continguous physical blocks in contiguous virtual blocks.
>>>   */
>>>  static unsigned
>>> -map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>> -        struct hugepage_info *hpi, int orig)
>>> +map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
>>> +          uint64_t *essential_memory __rte_unused, int orig)
>>>  {
>>>      int fd;
>>>      unsigned i;
>>>      void *virtaddr;
>>>      void *vma_addr = NULL;
>>>      size_t vma_len = 0;
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +    int node_id = -1;
>>> +    int essential_prev = 0;
>>> +    int oldpolicy;
>>> +    struct bitmask *oldmask = numa_allocate_nodemask();
>>> +    bool have_numa = true;
>>> +    unsigned long maxnode = 0;
>>> +
>>> +    /* Check if kernel supports NUMA. */
>>> +    if (numa_available() != 0) {
>>> +        RTE_LOG(DEBUG, EAL, "NUMA is not supported.\n");
>>> +        have_numa = false;
>>> +    }
>>> +
>>> +    if (orig && have_numa) {
>>> +        RTE_LOG(DEBUG, EAL, "Trying to obtain current memory policy.\n");
>>> +        if (get_mempolicy(&oldpolicy, oldmask->maskp,
>>> +                  oldmask->size + 1, 0, 0) < 0) {
>>> +            RTE_LOG(ERR, EAL,
>>> +                "Failed to get current mempolicy: %s. "
>>> +                "Assuming MPOL_DEFAULT.\n", strerror(errno));
>>> +            oldpolicy = MPOL_DEFAULT;
>>> +        }
>>> +        for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
>>> +            if (internal_config.socket_mem[i])
>>> +                maxnode = i + 1;
>>> +    }
>>> +#endif
>>>
>>>      for (i = 0; i < hpi->num_pages[0]; i++) {
>>>          uint64_t hugepage_sz = hpi->hugepage_sz;
>>>
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +        if (maxnode) {
>>> +            unsigned int j;
>>> +
>>> +            for (j = 0; j < maxnode; j++)
>>> +                if (essential_memory[j])
>>> +                    break;
>>> +
>>> +            if (j == maxnode) {
>>> +                node_id = (node_id + 1) % maxnode;
>>> +                while (!internal_config.socket_mem[node_id]) {
>>> +                    node_id++;
>>> +                    node_id %= maxnode;
>>> +                }
>>> +                essential_prev = 0;
>>> +            } else {
>>> +                node_id = j;
>>> +                essential_prev = essential_memory[j];
>>> +
>>> +                if (essential_memory[j] < hugepage_sz)
>>> +                    essential_memory[j] = 0;
>>> +                else
>>> +                    essential_memory[j] -= hugepage_sz;
>>> +            }
>>> +
>>> +            RTE_LOG(DEBUG, EAL,
>>> +                "Setting policy MPOL_PREFERRED for socket %d\n",
>>> +                node_id);
>>> +            numa_set_preferred(node_id);
>>> +        }
>>> +#endif
>>> +
>>>          if (orig) {
>>>              hugepg_tbl[i].file_id = i;
>>>              hugepg_tbl[i].size = hugepage_sz;
>>> @@ -422,7 +494,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>          if (fd < 0) {
>>>              RTE_LOG(DEBUG, EAL, "%s(): open failed: %s\n", __func__,
>>>                      strerror(errno));
>>> -            return i;
>>> +            goto out;
>>>          }
>>>
>>>          /* map the segment, and populate page tables,
>>> @@ -433,7 +505,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>              RTE_LOG(DEBUG, EAL, "%s(): mmap failed: %s\n", __func__,
>>>                      strerror(errno));
>>>              close(fd);
>>> -            return i;
>>> +            goto out;
>>>          }
>>>
>>>          if (orig) {
>>> @@ -458,7 +530,12 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>                  munmap(virtaddr, hugepage_sz);
>>>                  close(fd);
>>>                  unlink(hugepg_tbl[i].filepath);
>>> -                return i;
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +                if (maxnode)
>>> +                    essential_memory[node_id] =
>>> +                        essential_prev;
>>> +#endif
>>> +                goto out;
>>>              }
>>>              *(int *)virtaddr = 0;
>>>          }
>>> @@ -469,7 +546,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>              RTE_LOG(DEBUG, EAL, "%s(): Locking file failed:%s \n",
>>>                  __func__, strerror(errno));
>>>              close(fd);
>>> -            return i;
>>> +            goto out;
>>>          }
>>>
>>>          close(fd);
>>> @@ -478,6 +555,22 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>          vma_len -= hugepage_sz;
>>>      }
>>>
>>> +out:
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +    if (maxnode) {
>>> +        RTE_LOG(DEBUG, EAL,
>>> +            "Restoring previous memory policy: %d\n", oldpolicy);
>>> +        if (oldpolicy == MPOL_DEFAULT) {
>>> +            numa_set_localalloc();
>>> +        } else if (set_mempolicy(oldpolicy, oldmask->maskp,
>>> +                     oldmask->size + 1) < 0) {
>>> +            RTE_LOG(ERR, EAL, "Failed to restore mempolicy: %s\n",
>>> +                strerror(errno));
>>> +            numa_set_localalloc();
>>> +        }
>>> +    }
>>> +    numa_free_cpumask(oldmask);
>>> +#endif
>>>      return i;
>>>  }
>>>
>>> @@ -562,6 +655,11 @@ find_numasocket(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
>>>              if (hugepg_tbl[i].orig_va == va) {
>>>                  hugepg_tbl[i].socket_id = socket_id;
>>>                  hp_count++;
>>> +#ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>> +                RTE_LOG(DEBUG, EAL,
>>> +                    "Hugepage %s is on socket %d\n",
>>> +                    hugepg_tbl[i].filepath, socket_id);
>>> +#endif
>>>              }
>>>          }
>>>      }
>>> @@ -1000,6 +1098,11 @@ rte_eal_hugepage_init(void)
>>>
>>>      huge_register_sigbus();
>>>
>>> +    /* make a copy of socket_mem, needed for balanced allocation. */
>>> +    for (i = 0; i < RTE_MAX_NUMA_NODES; i++)
>>> +        memory[i] = internal_config.socket_mem[i];
>>> +
>>> +
>>>      /* map all hugepages and sort them */
>>>      for (i = 0; i < (int)internal_config.num_hugepage_sizes; i ++){
>>>          unsigned pages_old, pages_new;
>>> @@ -1017,7 +1120,8 @@ rte_eal_hugepage_init(void)
>>>
>>>          /* map all hugepages available */
>>>          pages_old = hpi->num_pages[0];
>>> -        pages_new = map_all_hugepages(&tmp_hp[hp_offset], hpi, 1);
>>> +        pages_new = map_all_hugepages(&tmp_hp[hp_offset], hpi,
>>> +                          memory, 1);
>>>          if (pages_new < pages_old) {
>>>              RTE_LOG(DEBUG, EAL,
>>>                  "%d not %d hugepages of size %u MB allocated\n",
>>> @@ -1060,7 +1164,7 @@ rte_eal_hugepage_init(void)
>>>                sizeof(struct hugepage_file), cmp_physaddr);
>>>
>>>          /* remap all hugepages */
>>> -        if (map_all_hugepages(&tmp_hp[hp_offset], hpi, 0) !=
>>> +        if (map_all_hugepages(&tmp_hp[hp_offset], hpi, NULL, 0) !=
>>>              hpi->num_pages[0]) {
>>>              RTE_LOG(ERR, EAL, "Failed to remap %u MB pages\n",
>>>                      (unsigned)(hpi->hugepage_sz / 0x100000));
>>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>>> index bcaf1b3..4fe22d1 100644
>>> --- a/mk/rte.app.mk
>>> +++ b/mk/rte.app.mk
>>> @@ -186,6 +186,9 @@ ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>>>  # The static libraries do not know their dependencies.
>>>  # So linking with static library requires explicit dependencies.
>>>  _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lrt
>>> +ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP)$(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),yy)
>>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lnuma
>>> +endif
>>>  _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)          += -lm
>>>  _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)          += -lrt
>>>  _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lm
>>>
>>
>>
>>
>>
>>