From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by dpdk.org (Postfix) with ESMTP id DFAB327D for ; Fri, 13 Jul 2018 18:05:27 +0200 (CEST) Received: by mail-ed1-f54.google.com with SMTP id r17-v6so24975325edo.13 for ; Fri, 13 Jul 2018 09:05:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=b4PgvnzxUcr+6hTtu1KHh8AyYtU/uM+8t59m0tJVcWY=; b=M1JqPn0vGM3vN6mCRe3IOEUsVay5poTdVjgCzitY6X2mSlEdIdmKeYAzcgwX5Sk01S vWqrM0fGWzclOYZiR/X8ABq6DZzicxFHI9JuFC3hFg/zMoZXAfbR98TkaUpEQWPxFpFM 4NjfUsLw8UQgnRTCbLZBCV5QgzuMNVxZuIiMKfJUU/RzuvIj9Y7EVswtXSP+S3InH+JF +y9oZEHQ17J6z1yO+Fdha9/ixAWOHcGyM2VE9dNdtHehHbt+GfD9VO55IYaFfnMWxglz JJqdpYOY2FaEBTxBwCu3HlItNOGsNfutb9PX7WhoRqPLndeV5p2Kmg5eDaDFAaCVmHYb UYvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=b4PgvnzxUcr+6hTtu1KHh8AyYtU/uM+8t59m0tJVcWY=; b=p+nlbBCDLHbWwaVWe6AP9j5wO5FBYOY1qe4V/UrzAT2KrVn0YOVjJy11MfUxV6+vW6 FIUUpsd6LtLg2MRzqkLSdlzEMFx8A/qmWezYJZAf+k96jcQhsiBjRBpcKNWT7hZegbD8 xwoFzB6szwgXchk5RQP2nUpXB+mSxYZP+0/rtkT4dl5LJgBZ+Xjw1FOgcaJVu37o4ABH 0Y4DW86dMg10ZPEajvaE+jbVl3alpYdVr7Oc3uv8YijlLhxXLP7/S+kbaPH0JUwX9DNb ybk3bKuFpfEZXUtn6O0ishZ5o55DBK0CeMpKaVbbx+cv8SB0dk/yz9FlWU/7DTE2EeJm lPRQ== X-Gm-Message-State: AOUpUlFFmdV1urxm1BYz63CfkvDrnCQUzQ3rtattDf2wNdfEX5CLTOFr hkM+eEBxPSvvP6hDIja2pChz/zY1P+SvTYnFiG9evw== X-Google-Smtp-Source: AAOMgpfMleuiamv/k1kUVC0C+9TbSKCbrpqk2xih95WMmczRb8105FL7VlSep3JNxQ2JHgb3YJVx5tX8OJtlc1Y6JDY= X-Received: by 2002:a50:86a1:: with SMTP id r30-v6mr7731354eda.138.1531497926339; Fri, 13 Jul 2018 09:05:26 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a50:b194:0:0:0:0:0 with HTTP; Fri, 13 Jul 2018 09:05:25 -0700 (PDT) In-Reply-To: References: From: Alejandro Lucero Date: Fri, 13 Jul 2018 17:05:25 +0100 Message-ID: To: Anatoly Burakov Cc: dev , Thomas Monjalon , srinath.mannam@broadcom.com, scott.branden@broadcom.com, Ajit Khaparde Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC 03/11] malloc: index heaps using heap ID rather than NUMA node X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jul 2018 16:05:28 -0000 On Fri, Jul 6, 2018 at 2:17 PM, Anatoly Burakov 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. > > Signed-off-by: Anatoly Burakov > --- > config/common_base | 1 + > lib/librte_eal/common/eal_common_memzone.c | 46 ++++++++++------ > .../common/include/rte_eal_memconfig.h | 4 +- > lib/librte_eal/common/malloc_heap.c | 53 ++++++++++++------- > lib/librte_eal/common/rte_malloc.c | 28 ++++++---- > 5 files changed, 84 insertions(+), 48 deletions(-) > > diff --git a/config/common_base b/config/common_base > index fcf3a1f6f..b0e3937e0 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/lib/librte_eal/common/eal_common_memzone.c > b/lib/librte_eal/common/eal_common_memzone.c > index faa3b0615..25c56052c 100644 > --- a/lib/librte_eal/common/eal_common_memzone.c > +++ b/lib/librte_eal/common/eal_common_memzone.c > @@ -52,6 +52,26 @@ memzone_lookup_thread_unsafe(const char *name) > return NULL; > } > > +static size_t > +heap_max_free_elem(unsigned int heap_idx, unsigned int align) > +{ > + struct rte_malloc_socket_stats stats; > + struct rte_mem_config *mcfg; > + size_t len; > + > + /* get pointer to global configuration */ > + mcfg = rte_eal_get_configuration()->mem_config; > + > + malloc_heap_get_stats(&mcfg->malloc_heaps[heap_idx], &stats); > + > + len = stats.greatest_free_size; > + > + if (len < MALLOC_ELEM_OVERHEAD + align) > + return 0; > + > + return len - MALLOC_ELEM_OVERHEAD - align; > +} > + > > /* This function will return the greatest free block if a heap has been > * specified. If no heap has been specified, it will return the heap and > This description should be updated as no heap/socket is used now. > @@ -59,29 +79,23 @@ memzone_lookup_thread_unsafe(const char *name) > static size_t > find_heap_max_free_elem(int *s, unsigned align) > { > - struct rte_mem_config *mcfg; > - struct rte_malloc_socket_stats stats; > - int i, socket = *s; > + unsigned int idx; > + int socket = *s; > size_t len = 0; > > - /* get pointer to global configuration */ > - mcfg = rte_eal_get_configuration()->mem_config; > - > - for (i = 0; i < RTE_MAX_NUMA_NODES; i++) { > - if ((socket != SOCKET_ID_ANY) && (socket != i)) > + for (idx = 0; idx < rte_socket_count(); idx++) { > + int cur_socket = rte_socket_id_by_idx(idx); > + if ((socket != SOCKET_ID_ANY) && (socket != cur_socket)) > continue; > > - malloc_heap_get_stats(&mcfg->malloc_heaps[i], &stats); > - if (stats.greatest_free_size > len) { > - len = stats.greatest_free_size; > - *s = i; > + size_t cur_len = heap_max_free_elem(idx, align); > + if (cur_len > len) { > + len = cur_len; > + *s = cur_socket; > } > } > > - if (len < MALLOC_ELEM_OVERHEAD + align) > - return 0; > - > - return len - MALLOC_ELEM_OVERHEAD - align; > + return len; > Is it worth to set *s to some safe value if no space at all? > } > > static const struct rte_memzone * > diff --git a/lib/librte_eal/common/include/rte_eal_memconfig.h > b/lib/librte_eal/common/include/rte_eal_memconfig.h > index 4e8720ba6..7e03196a6 100644 > --- a/lib/librte_eal/common/include/rte_eal_memconfig.h > +++ b/lib/librte_eal/common/include/rte_eal_memconfig.h > @@ -71,8 +71,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/malloc_heap.c b/lib/librte_eal/common/ > malloc_heap.c > index 8a1f54905..e7e1838b1 100644 > --- a/lib/librte_eal/common/malloc_heap.c > +++ b/lib/librte_eal/common/malloc_heap.c > @@ -93,9 +93,10 @@ 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; > > - heap = &mcfg->malloc_heaps[msl->socket_id]; > + heap_idx = rte_socket_idx_by_id(msl->socket_id); > + heap = &mcfg->malloc_heaps[heap_idx]; > > /* msl is const, so find it */ > msl_idx = msl - mcfg->memsegs; > @@ -494,14 +495,20 @@ alloc_more_mem_on_socket(struct malloc_heap *heap, > size_t size, int socket, > > /* this will try lower page sizes first */ > static void * > -heap_alloc_on_socket(const char *type, size_t size, int socket, > - unsigned int flags, size_t align, size_t bound, bool > contig) > +malloc_heap_alloc_on_heap_id(const char *type, size_t size, > + unsigned int heap_id, unsigned int flags, size_t align, > + size_t bound, bool contig) > { > struct rte_mem_config *mcfg = rte_eal_get_configuration()-> > mem_config; > - struct malloc_heap *heap = &mcfg->malloc_heaps[socket]; > + struct malloc_heap *heap = &mcfg->malloc_heaps[heap_id]; > unsigned int size_flags = flags & ~RTE_MEMZONE_SIZE_HINT_ONLY; > + int socket_id; > void *ret; > > + /* return NULL if size is 0 or alignment is not power-of-2 */ > + if (size == 0 || (align && !rte_is_power_of_2(align))) > + return NULL; > + > rte_spinlock_lock(&(heap->lock)); > > align = align == 0 ? 1 : align; > @@ -521,8 +528,13 @@ heap_alloc_on_socket(const char *type, size_t size, > int socket, > if (ret != NULL) > goto alloc_unlock; > > - if (!alloc_more_mem_on_socket(heap, size, socket, flags, align, > bound, > - contig)) { > + socket_id = rte_socket_id_by_idx(heap_id); > + /* if socket ID is invalid, this is an external heap */ > + if (socket_id < 0) > + goto alloc_unlock; > A log message would help to know what is going on. It is costly, but better than hidden problem. > + > + if (!alloc_more_mem_on_socket(heap, size, socket_id, flags, align, > + bound, contig)) { > ret = heap_alloc(heap, type, size, flags, align, bound, > contig); > > /* this should have succeeded */ > @@ -538,13 +550,9 @@ void * > malloc_heap_alloc(const char *type, size_t size, int socket_arg, > unsigned int flags, size_t align, size_t bound, bool > contig) > { > - int socket, i, cur_socket; > + int socket, heap_id, i; > void *ret; > > - /* return NULL if size is 0 or alignment is not power-of-2 */ > - if (size == 0 || (align && !rte_is_power_of_2(align))) > - return NULL; > - > if (!rte_eal_has_hugepages()) > socket_arg = SOCKET_ID_ANY; > > @@ -557,18 +565,23 @@ malloc_heap_alloc(const char *type, size_t size, int > socket_arg, > if (socket >= RTE_MAX_NUMA_NODES) > return NULL; > > - ret = heap_alloc_on_socket(type, size, socket, flags, align, bound, > - contig); > + /* turn socket ID into heap ID */ > + heap_id = rte_socket_idx_by_id(socket); > + /* if heap id is invalid, it's a non-existent NUMA node */ > + if (heap_id < 0) > same than above. A log would help. > + return NULL; > + > + ret = malloc_heap_alloc_on_heap_id(type, size, heap_id, flags, > align, > + bound, contig); > if (ret != NULL || socket_arg != SOCKET_ID_ANY) > return ret; > > /* try other heaps */ > for (i = 0; i < (int) rte_socket_count(); i++) { > - cur_socket = rte_socket_id_by_idx(i); > - if (cur_socket == socket) > + if (i == heap_id) > continue; > - ret = heap_alloc_on_socket(type, size, cur_socket, flags, > - align, bound, contig); > + ret = malloc_heap_alloc_on_heap_id(type, size, i, flags, > align, > + bound, contig); > if (ret != NULL) > return ret; > } > @@ -788,7 +801,7 @@ malloc_heap_resize(struct malloc_elem *elem, size_t > size) > } > > /* > - * Function to retrieve data for heap on given socket > + * Function to retrieve data for a given heap > */ > int > malloc_heap_get_stats(struct malloc_heap *heap, > @@ -826,7 +839,7 @@ malloc_heap_get_stats(struct malloc_heap *heap, > } > > /* > - * Function to retrieve data for heap on given socket > + * Function to retrieve data for a given heap > */ > void > malloc_heap_dump(struct malloc_heap *heap, FILE *f) > diff --git a/lib/librte_eal/common/rte_malloc.c > b/lib/librte_eal/common/rte_malloc.c > index b51a6d111..4387bc494 100644 > --- a/lib/librte_eal/common/rte_malloc.c > +++ b/lib/librte_eal/common/rte_malloc.c > @@ -152,11 +152,17 @@ rte_malloc_get_socket_stats(int socket, > struct rte_malloc_socket_stats *socket_stats) > { > struct rte_mem_config *mcfg = rte_eal_get_configuration()-> > mem_config; > + int heap_idx; > > if (socket >= RTE_MAX_NUMA_NODES || socket < 0) > return -1; > > - return malloc_heap_get_stats(&mcfg->malloc_heaps[socket], > socket_stats); > + heap_idx = rte_socket_idx_by_id(socket); > + if (heap_idx < 0) > + return -1; > + > + return malloc_heap_get_stats(&mcfg->malloc_heaps[heap_idx], > + socket_stats); > } > > /* > @@ -168,10 +174,9 @@ rte_malloc_dump_heaps(FILE *f) > struct rte_mem_config *mcfg = rte_eal_get_configuration()-> > mem_config; > unsigned int idx; > > - for (idx = 0; idx < rte_socket_count(); idx++) { > - unsigned int socket = rte_socket_id_by_idx(idx); > - fprintf(f, "Heap on socket %i:\n", socket); > - malloc_heap_dump(&mcfg->malloc_heaps[socket], f); > + for (idx = 0; idx < RTE_MAX_HEAPS; idx++) { > + fprintf(f, "Heap id: %u\n", idx); > + malloc_heap_dump(&mcfg->malloc_heaps[idx], f); > } > > } > @@ -182,14 +187,17 @@ rte_malloc_dump_heaps(FILE *f) > void > rte_malloc_dump_stats(FILE *f, __rte_unused const char *type) > { > - unsigned int socket; > + struct rte_mem_config *mcfg = rte_eal_get_configuration()-> > mem_config; > + unsigned int heap_id; > struct rte_malloc_socket_stats sock_stats; > + > /* Iterate through all initialised heaps */ > - for (socket=0; socket< RTE_MAX_NUMA_NODES; socket++) { > - if ((rte_malloc_get_socket_stats(socket, &sock_stats) < > 0)) > - continue; > + for (heap_id = 0; heap_id < RTE_MAX_HEAPS; heap_id++) { > + struct malloc_heap *heap = &mcfg->malloc_heaps[heap_id]; > > - fprintf(f, "Socket:%u\n", socket); > + malloc_heap_get_stats(heap, &sock_stats); > + > + fprintf(f, "Heap id:%u\n", heap_id); > fprintf(f, "\tHeap_size:%zu,\n", > sock_stats.heap_totalsz_bytes); > fprintf(f, "\tFree_size:%zu,\n", > sock_stats.heap_freesz_bytes); > fprintf(f, "\tAlloc_size:%zu,\n", > sock_stats.heap_allocsz_bytes); > -- > 2.17.1 >