From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CA11BA0548; Thu, 4 Nov 2021 09:54:19 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8DFEA40E5A; Thu, 4 Nov 2021 09:54:19 +0100 (CET) Received: from smtpservice.6wind.com (host.78.145.23.62.rev.coltfrance.com [62.23.145.78]) by mails.dpdk.org (Postfix) with ESMTP id 8BEC340DFD; Thu, 4 Nov 2021 09:54:18 +0100 (CET) Received: from 6wind.com (unknown [10.16.18.213]) by smtpservice.6wind.com (Postfix) with ESMTP id 688536001B; Thu, 4 Nov 2021 09:54:18 +0100 (CET) Date: Thu, 4 Nov 2021 09:54:18 +0100 From: Olivier Matz To: David Marchand Cc: Anatoly Burakov , dev , Ilyes Ben Hamouda , dpdk stable Message-ID: References: <20211029094929.29864-1-olivier.matz@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH] eal: fix mem alloc from control thread if socket 0 is unused X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Nov 03, 2021 at 09:26:02PM +0100, David Marchand wrote: > On Fri, Oct 29, 2021 at 11:49 AM Olivier Matz wrote: > > > > From: Ilyes Ben Hamouda > > > > When using rte_malloc() from a control thread, the used heap is the one > > from numa socket 0, which may not have available memory. > > > > Fix this by selecting the first socket which has available memory. > > > > Note: malloc_get_numa_socket() is only used from one .c file, so move > > it there, and remove the inline keyword. > > > > Fixes: b94580d6887e ("malloc: avoid unknown socket id") > > Cc: stable@dpdk.org > > > > Signed-off-by: Ilyes Ben Hamouda > > Signed-off-by: Olivier Matz > > --- > > lib/eal/common/malloc_heap.c | 20 ++++++++++++++++++++ > > lib/eal/common/malloc_heap.h | 11 ----------- > > 2 files changed, 20 insertions(+), 11 deletions(-) > > > > diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c > > index ee400f38ec..6eff9a2284 100644 > > --- a/lib/eal/common/malloc_heap.c > > +++ b/lib/eal/common/malloc_heap.c > > @@ -694,6 +694,26 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size, > > return ret; > > } > > > > +static unsigned int > > +malloc_get_numa_socket(void) > > +{ > > + const struct internal_config *conf = eal_get_internal_configuration(); > > + unsigned int socket_id = rte_socket_id(); > > + unsigned int idx; > > + > > + if (socket_id != (unsigned int)SOCKET_ID_ANY) > > + return socket_id; > > + > > Strictly speaking, this affects more than control threads. > > socket_id == SOCKET_ID_ANY is possible if current thread is started on > cores from 2 different sockets. > See thread_update_affinity(), which calls eal_cpuset_socket_id(). > > As a consequence, this change affects lcores too like if a lcore is > pinned on cores from two sockets like --lcores 0@(1,2) with core 1 on > numa1 and core 2 on numa2 (giving this odd example on purpose). > Previously, all allocations from this lcore would end up on numa0, > regardless of memory availability. > So this change fixes allocations for this odd setup too. I didn't know this was possible (and still wonder in which case it can be useful). But yes, I can send a new version with an updated title and commit log. What about this one below? eal: fix mem alloc from thread having unknown socket id When using rte_malloc() from a thread which is not bound to a numa socket (the typical case is a control thread, but it can also happen on a dataplane thread if its cpu affinity is on cores attached to several sockets), the used heap is the one from numa socket 0, which may not have available memory. Fix this by selecting the first socket which has available memory. Note: malloc_get_numa_socket() is only used from one .c file, so move it there, and remove the inline keyword. > > > + /* for control threads, return first socket where memory is available */ > > + for (idx = 0; idx < rte_socket_count(); idx++) { > > + socket_id = rte_socket_id_by_idx(idx); > > + if (conf->socket_mem[socket_id] != 0) > > + return socket_id; > > + } > > We could look at current thread cpu affinity to check to which sockets > it is bound to (like what is done in eal_cpuset_socket_id()). > But that would make the code rather complex and the setups in which it > helps are surely even even more odd than what I mentionned above. > > Your proposed heuristic looks fine to me, let's go with it if nobody objects. I also think the current heuristic covers the real-life use cases. Thanks for the review Olivier > > > > + > > + return rte_socket_id_by_idx(0); > > +} > > + > > void * > > malloc_heap_alloc(const char *type, size_t size, int socket_arg, > > unsigned int flags, size_t align, size_t bound, bool contig) > > diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h > > index 3a6ec6ecf0..3a29d024b4 100644 > > --- a/lib/eal/common/malloc_heap.h > > +++ b/lib/eal/common/malloc_heap.h > > @@ -33,17 +33,6 @@ struct malloc_heap { > > char name[RTE_HEAP_NAME_MAX_LEN]; > > } __rte_cache_aligned; > > > > -static inline unsigned > > -malloc_get_numa_socket(void) > > -{ > > - unsigned socket_id = rte_socket_id(); > > - > > - if (socket_id == (unsigned)SOCKET_ID_ANY) > > - return 0; > > - > > - return socket_id; > > -} > > - > > void * > > malloc_heap_alloc(const char *type, size_t size, int socket, unsigned int flags, > > size_t align, size_t bound, bool contig); > > -- > > 2.30.2 > > > > > -- > David Marchand >