From patchwork Tue Feb 14 12:54:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 64958 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 52ED23858C5E for ; Tue, 14 Feb 2023 12:54:58 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 38EB53858D1E for ; Tue, 14 Feb 2023 12:54:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 38EB53858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.97,296,1669104000"; d="scan'208,223";a="100705477" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 14 Feb 2023 04:54:40 -0800 IronPort-SDR: fhDHZgCA3cQlSr743XV1Q8G5yuNF46MQeJragT84kAHJB6skYCMKu0XqlF8IjTeoDJcHcod659 8nqom898V9YvEzu8azqUm24khOM4tps/TRNR2aRlFKOWxG11i/7oAsbcA8Tos3PZEgQNh83wvD k0L8JUrjGAlZsDOh0F9FDueA62NPGnn5wFXtKneA3RMFmp5HVAM1zTYn4+xVQJXcotXf/1KFVO M9O4GzJt1j467vI8AccpLfJSzU1FV0o0UvNmQcVcYeVAa4PadHxzB8xTkVijqN3jsRJGOR9/UL 2Mc= From: Thomas Schwinge To: Andrew Stubbs , CC: Jakub Jelinek , Tobias Burnus , Tom de Vries Subject: [og12] In 'libgomp/allocator.c:omp_realloc', route 'free' through 'MEMSPACE_FREE' (was: [PATCH] libgomp, OpenMP, nvptx: Low-latency memory allocator) In-Reply-To: <93ae0df7-6cb9-40de-81e6-768029ca49fc@codesourcery.com> References: <25ad524d-f0d6-1970-b8e9-9b11b6cde68b@codesourcery.com> <42c70624-2b10-340c-8945-601203768d48@suse.de> <664653d3-cf64-b800-6ffb-c27e50dc15bf@suse.de> <5e75a64c-a8d3-2d2a-162a-a3ea79358b48@suse.de> <3e39c39d-2753-f08c-5fb7-85051cafab85@suse.de> <985755ee-f7bd-7d47-1ea0-1ca980117f6d@codesourcery.com> <93ae0df7-6cb9-40de-81e6-768029ca49fc@codesourcery.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Tue, 14 Feb 2023 13:54:30 +0100 Message-ID: <87h6voz9tl.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi Andrew! On 2022-01-13T11:13:51+0000, Andrew Stubbs wrote: > Updated patch: this version fixes some missed cases of malloc in the > realloc implementation. Right, and as it seems I've run into another issue: a stray 'free'. > --- a/libgomp/allocator.c > +++ b/libgomp/allocator.c Re 'omp_realloc': > @@ -660,9 +709,10 @@ retry: > gomp_mutex_unlock (&allocator_data->lock); > #endif > if (prev_size) > - new_ptr = realloc (data->ptr, new_size); > + new_ptr = MEMSPACE_REALLOC (allocator_data->memspace, data->ptr, > + data->size, new_size); > else > - new_ptr = malloc (new_size); > + new_ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size); > if (new_ptr == NULL) > { > #ifdef HAVE_SYNC_BUILTINS > @@ -690,7 +740,11 @@ retry: > && (free_allocator_data == NULL > || free_allocator_data->pool_size == ~(uintptr_t) 0)) > { > - new_ptr = realloc (data->ptr, new_size); > + omp_memspace_handle_t memspace __attribute__((unused)) > + = (allocator_data > + ? allocator_data->memspace > + : predefined_alloc_mapping[allocator]); > + new_ptr = MEMSPACE_REALLOC (memspace, data->ptr, data->size, new_size); > if (new_ptr == NULL) > goto fail; > ret = (char *) new_ptr + sizeof (struct omp_mem_header); > @@ -701,7 +755,11 @@ retry: > } > else > { > - new_ptr = malloc (new_size); > + omp_memspace_handle_t memspace __attribute__((unused)) > + = (allocator_data > + ? allocator_data->memspace > + : predefined_alloc_mapping[allocator]); > + new_ptr = MEMSPACE_ALLOC (memspace, new_size); > if (new_ptr == NULL) > goto fail; > } > @@ -735,32 +793,35 @@ retry: | free (data->ptr); > return ret; I run into a SIGSEGV if a non-'malloc'-based allocation is 'free'd here. The attached "In 'libgomp/allocator.c:omp_realloc', route 'free' through 'MEMSPACE_FREE'" appears to resolve my issue, but not yet regression-tested. Does that look correct to you? Or, instead of invoking 'MEMSPACE_FREE', should we scrap the 'used_pool_size' bookkeeping here, and just invoke 'omp_free' instead? --- libgomp/allocator.c +++ libgomp/allocator.c @@ -842,19 +842,7 @@ retry: if (old_size - old_alignment < size) size = old_size - old_alignment; memcpy (ret, ptr, size); - if (__builtin_expect (free_allocator_data - && free_allocator_data->pool_size < ~(uintptr_t) 0, 0)) - { -#ifdef HAVE_SYNC_BUILTINS - __atomic_add_fetch (&free_allocator_data->used_pool_size, -data->size, - MEMMODEL_RELAXED); -#else - gomp_mutex_lock (&free_allocator_data->lock); - free_allocator_data->used_pool_size -= data->size; - gomp_mutex_unlock (&free_allocator_data->lock); -#endif - } - free (data->ptr); + ialias_call (omp_free) (ptr, free_allocator); return ret; (I've not yet analyzed whether that's completely equivalent.) Note that this likewise applies to the current upstream submission: "libgomp, nvptx: low-latency memory allocator". Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 From d49d0b9dc4f96c496afb2d5caac4addb382fdf39 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Tue, 14 Feb 2023 13:35:03 +0100 Subject: [PATCH] In 'libgomp/allocator.c:omp_realloc', route 'free' through 'MEMSPACE_FREE' ... to not run into a SIGSEGV if a non-'malloc'-based allocation is 'free'd here. Fix-up for og12 commit c5d1d7651297a273321154a5fe1b01eba9dcf604 "libgomp, nvptx: low-latency memory allocator". libgomp/ * allocator.c (omp_realloc): Route 'free' through 'MEMSPACE_FREE'. --- libgomp/allocator.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/libgomp/allocator.c b/libgomp/allocator.c index 05b323d458e2..ba9a4e17cc20 100644 --- a/libgomp/allocator.c +++ b/libgomp/allocator.c @@ -854,7 +854,17 @@ retry: gomp_mutex_unlock (&free_allocator_data->lock); #endif } - free (data->ptr); + { + omp_memspace_handle_t was_memspace __attribute__((unused)) + = (free_allocator_data + ? free_allocator_data->memspace + : predefined_alloc_mapping[free_allocator]); + int was_pinned __attribute__((unused)) + = (free_allocator_data + ? free_allocator_data->pinned + : free_allocator == ompx_pinned_mem_alloc); + MEMSPACE_FREE (was_memspace, data->ptr, data->size, was_pinned); + } return ret; fail: -- 2.39.1