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 F13A841D5D; Fri, 24 Feb 2023 09:17:01 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6822940EF1; Fri, 24 Feb 2023 09:17:01 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 2E4DE40693 for ; Fri, 24 Feb 2023 09:17:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677226619; 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=hVMDxBnr7rSFX7BBtxoRRpJwzYVNX3doTYQqYsoTvC3O2pVsDq/rdb96XExqksfx1zSMeB im9gg09Ad5EfFP5G2IW87SqqXzpMrYDIXXuI7GqW47QB2s82K9FECzTeyQEYxPac1GOClS ++2JQZK7wPNI2lfpDwksz0y9YSoF1us= 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-669-eAVmm8YxPV6srYQOjMJzDQ-1; Fri, 24 Feb 2023 03:16:55 -0500 X-MC-Unique: eAVmm8YxPV6srYQOjMJzDQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CB2D9800B23; Fri, 24 Feb 2023 08:16:54 +0000 (UTC) Received: from dmarchan.redhat.com (unknown [10.45.224.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id 03B8B492B07; Fri, 24 Feb 2023 08:16:53 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: thomas@monjalon.net, Anatoly Burakov Subject: [PATCH 01/14] malloc: rework heap lock handling Date: Fri, 24 Feb 2023 09:16:29 +0100 Message-Id: <20230224081642.2566619-2-david.marchand@redhat.com> In-Reply-To: <20230224081642.2566619-1-david.marchand@redhat.com> References: <20230224081642.2566619-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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