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 B33D241D5F; Fri, 24 Feb 2023 16:11:57 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 508D540697; Fri, 24 Feb 2023 16:11:57 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id B53EB40693 for ; Fri, 24 Feb 2023 16:11:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677251515; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pys5mz0G2mUZukXwaKteJY89qQfoORUAaHkqtbFYt4A=; b=fqtkNfINJapdM1HqRgVoiaYHCb72Vget+V97khwJU5U6Ph6H56cwfmZUabMpNSKrhJaCPZ wKQPxbz0ANp7/Z7fgwyj8gHJ0Rs4CdZJxShnM47MrPcl1OAHSy3IDiOPK6/qqD/bGya7/k JcnZITHw74kQBngBPSmWzUcV/X/sK5U= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-622-apF7A6HFOt-mMYtK6Tb41Q-1; Fri, 24 Feb 2023 10:11:53 -0500 X-MC-Unique: apF7A6HFOt-mMYtK6Tb41Q-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6729C1871D94; Fri, 24 Feb 2023 15:11:53 +0000 (UTC) Received: from dmarchan.redhat.com (unknown [10.45.224.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id A624840C10FA; Fri, 24 Feb 2023 15:11:52 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: thomas@monjalon.net, Anatoly Burakov Subject: [PATCH v2 01/20] malloc: rework heap lock handling Date: Fri, 24 Feb 2023 16:11:24 +0100 Message-Id: <20230224151143.3274897-2-david.marchand@redhat.com> In-Reply-To: <20230224151143.3274897-1-david.marchand@redhat.com> References: <20230224081642.2566619-1-david.marchand@redhat.com> <20230224151143.3274897-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true 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 Move all heap->lock manipulation to malloc_heap.c to have a single location where to look at and make higher level code unaware of this locking constraint. The destroy helper has been reworked to zero all the heap object but leave the lock untouched. The heap lock is then released through the standard API. This makes the code less scary even though, at this point of its life, the heap object is probably referenced only by the caller. Signed-off-by: David Marchand --- lib/eal/common/malloc_heap.c | 45 +++++++++++++++++++++++++++--------- lib/eal/common/rte_malloc.c | 10 -------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c index d7c410b786..cc84dce672 100644 --- a/lib/eal/common/malloc_heap.c +++ b/lib/eal/common/malloc_heap.c @@ -1292,6 +1292,8 @@ int malloc_heap_add_external_memory(struct malloc_heap *heap, struct rte_memseg_list *msl) { + rte_spinlock_lock(&heap->lock); + /* erase contents of new memory */ memset(msl->base_va, 0, msl->len); @@ -1308,6 +1310,11 @@ malloc_heap_add_external_memory(struct malloc_heap *heap, eal_memalloc_mem_event_notify(RTE_MEM_EVENT_ALLOC, msl->base_va, msl->len); + /* mark it as heap segment */ + msl->heap = 1; + + rte_spinlock_unlock(&heap->lock); + return 0; } @@ -1315,7 +1322,12 @@ int malloc_heap_remove_external_memory(struct malloc_heap *heap, void *va_addr, size_t len) { - struct malloc_elem *elem = heap->first; + struct malloc_elem *elem; + int ret = -1; + + rte_spinlock_lock(&heap->lock); + + elem = heap->first; /* find element with specified va address */ while (elem != NULL && elem != va_addr) { @@ -1323,20 +1335,24 @@ malloc_heap_remove_external_memory(struct malloc_heap *heap, void *va_addr, /* stop if we've blown past our VA */ if (elem > (struct malloc_elem *)va_addr) { rte_errno = ENOENT; - return -1; + goto out; } } /* check if element was found */ if (elem == NULL || elem->msl->len != len) { rte_errno = ENOENT; - return -1; + goto out; } /* if element's size is not equal to segment len, segment is busy */ if (elem->state == ELEM_BUSY || elem->size != len) { rte_errno = EBUSY; - return -1; + goto out; } - return destroy_elem(elem, len); + ret = destroy_elem(elem, len); + +out: + rte_spinlock_unlock(&heap->lock); + return ret; } int @@ -1372,23 +1388,30 @@ malloc_heap_create(struct malloc_heap *heap, const char *heap_name) int malloc_heap_destroy(struct malloc_heap *heap) { + int ret = -1; + + rte_spinlock_lock(&heap->lock); + if (heap->alloc_count != 0) { RTE_LOG(ERR, EAL, "Heap is still in use\n"); rte_errno = EBUSY; - return -1; + goto fail; } if (heap->first != NULL || heap->last != NULL) { RTE_LOG(ERR, EAL, "Heap still contains memory segments\n"); rte_errno = EBUSY; - return -1; + goto fail; } if (heap->total_size != 0) RTE_LOG(ERR, EAL, "Total size not zero, heap is likely corrupt\n"); - /* after this, the lock will be dropped */ - memset(heap, 0, sizeof(*heap)); - - return 0; + RTE_BUILD_BUG_ON(offsetof(struct malloc_heap, lock) != 0); + memset(RTE_PTR_ADD(heap, sizeof(heap->lock)), 0, + sizeof(*heap) - sizeof(heap->lock)); + ret = 0; +fail: + rte_spinlock_unlock(&heap->lock); + return ret; } int diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c index d39870bf3c..4f500892f2 100644 --- a/lib/eal/common/rte_malloc.c +++ b/lib/eal/common/rte_malloc.c @@ -436,10 +436,7 @@ rte_malloc_heap_memory_add(const char *heap_name, void *va_addr, size_t len, goto unlock; } - rte_spinlock_lock(&heap->lock); ret = malloc_heap_add_external_memory(heap, msl); - msl->heap = 1; /* mark it as heap segment */ - rte_spinlock_unlock(&heap->lock); unlock: rte_mcfg_mem_write_unlock(); @@ -482,9 +479,7 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len) goto unlock; } - rte_spinlock_lock(&heap->lock); ret = malloc_heap_remove_external_memory(heap, va_addr, len); - rte_spinlock_unlock(&heap->lock); if (ret != 0) goto unlock; @@ -655,12 +650,7 @@ rte_malloc_heap_destroy(const char *heap_name) goto unlock; } /* sanity checks done, now we can destroy the heap */ - rte_spinlock_lock(&heap->lock); ret = malloc_heap_destroy(heap); - - /* if we failed, lock is still active */ - if (ret < 0) - rte_spinlock_unlock(&heap->lock); unlock: rte_mcfg_mem_write_unlock(); -- 2.39.2