From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id B692112008 for ; Mon, 19 Mar 2018 18:46:18 +0100 (CET) Received: from lfbn-lil-1-702-109.w81-254.abo.wanadoo.fr ([81.254.39.109] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1exyrt-0007BK-D1; Mon, 19 Mar 2018 18:46:46 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 19 Mar 2018 18:46:11 +0100 Date: Mon, 19 Mar 2018 18:46:11 +0100 From: Olivier Matz To: Anatoly Burakov Cc: dev@dpdk.org, keith.wiles@intel.com, jianfeng.tan@intel.com, andras.kovacs@ericsson.com, laszlo.vadkeri@ericsson.com, benjamin.walker@intel.com, bruce.richardson@intel.com, thomas@monjalon.net, konstantin.ananyev@intel.com, kuralamudhan.ramakrishnan@intel.com, louise.m.daly@intel.com, nelio.laranjeiro@6wind.com, yskoh@mellanox.com, pepperjo@japf.ch, jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com Message-ID: <20180319174611.p2s5q6mow3b6wbcq@platinum> References: <79bd4a81a4a85908a011a5fffb33b881dd3a93c9.1520083504.git.anatoly.burakov@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <79bd4a81a4a85908a011a5fffb33b881dd3a93c9.1520083504.git.anatoly.burakov@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 17/41] eal: enable memory hotplug support in rte_malloc 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: Mon, 19 Mar 2018 17:46:18 -0000 On Sat, Mar 03, 2018 at 01:46:05PM +0000, Anatoly Burakov wrote: > This set of changes enables rte_malloc to allocate and free memory > as needed. The way it works is, first malloc checks if there is > enough memory already allocated to satisfy user's request. If there > isn't, we try and allocate more memory. The reverse happens with > free - we free an element, check its size (including free element > merging due to adjacency) and see if it's bigger than hugepage > size and that its start and end span a hugepage or more. Then we > remove the area from malloc heap (adjusting element lengths where > appropriate), and deallocate the page. > > For legacy mode, runtime alloc/free of pages is disabled. > > It is worth noting that memseg lists are being sorted by page size, > and that we try our best to satisfy user's request. That is, if > the user requests an element from a 2MB page memory, we will check > if we can satisfy that request from existing memory, if not we try > and allocate more 2MB pages. If that fails and user also specified > a "size is hint" flag, we then check other page sizes and try to > allocate from there. If that fails too, then, depending on flags, > we may try allocating from other sockets. In other words, we try > our best to give the user what they asked for, but going to other > sockets is last resort - first we try to allocate more memory on > the same socket. > > Signed-off-by: Anatoly Burakov [...] > @@ -123,48 +125,356 @@ find_suitable_element(struct malloc_heap *heap, size_t size, > * scan fails. Once the new memseg is added, it re-scans and should return > * the new element after releasing the lock. > */ > -void * > -malloc_heap_alloc(struct malloc_heap *heap, > - const char *type __attribute__((unused)), size_t size, unsigned flags, > - size_t align, size_t bound) > +static void * > +heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size, > + unsigned int flags, size_t align, size_t bound) > { > struct malloc_elem *elem; > > size = RTE_CACHE_LINE_ROUNDUP(size); > align = RTE_CACHE_LINE_ROUNDUP(align); > > - rte_spinlock_lock(&heap->lock); > - > elem = find_suitable_element(heap, size, flags, align, bound); > if (elem != NULL) { > elem = malloc_elem_alloc(elem, size, align, bound); > + > /* increase heap's count of allocated elements */ > heap->alloc_count++; > } > - rte_spinlock_unlock(&heap->lock); > > return elem == NULL ? NULL : (void *)(&elem[1]); > } The comment on top of the function says "after releasing the lock" but it seems it's not relevant anymore because the lock is removed. [...] > int > malloc_heap_free(struct malloc_elem *elem) > { > struct malloc_heap *heap; > - struct malloc_elem *ret; > + void *start, *aligned_start, *end, *aligned_end; > + size_t len, aligned_len; > + struct rte_memseg_list *msl; > + int n_pages, page_idx, max_page_idx, ret; > > if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY) > return -1; > > /* elem may be merged with previous element, so keep heap address */ > heap = elem->heap; > + msl = elem->msl; > > rte_spinlock_lock(&(heap->lock)); > > - ret = malloc_elem_free(elem); > + elem = malloc_elem_free(elem); > > - rte_spinlock_unlock(&(heap->lock)); > + /* anything after this is a bonus */ > + ret = 0; > + The fact that there was previously 2 rte_spinlock_unlock() calls looks strange to me. Is there something wrong in a previous patch?