From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Content-Type: multipart/mixed; boundary="=-=-=" 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 List-Id: --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 =3D realloc (data->ptr, new_size); > + new_ptr =3D MEMSPACE_REALLOC (allocator_data->memspace, data->ptr, > + data->size, new_size); > else > - new_ptr =3D malloc (new_size); > + new_ptr =3D MEMSPACE_ALLOC (allocator_data->memspace, new_size); > if (new_ptr =3D=3D NULL) > { > #ifdef HAVE_SYNC_BUILTINS > @@ -690,7 +740,11 @@ retry: > && (free_allocator_data =3D=3D NULL > || free_allocator_data->pool_size =3D=3D ~(uintptr_t) 0)) > { > - new_ptr =3D realloc (data->ptr, new_size); > + omp_memspace_handle_t memspace __attribute__((unused)) > + =3D (allocator_data > + ? allocator_data->memspace > + : predefined_alloc_mapping[allocator]); > + new_ptr =3D MEMSPACE_REALLOC (memspace, data->ptr, data->size, new= _size); > if (new_ptr =3D=3D NULL) > goto fail; > ret =3D (char *) new_ptr + sizeof (struct omp_mem_header); > @@ -701,7 +755,11 @@ retry: > } > else > { > - new_ptr =3D malloc (new_size); > + omp_memspace_handle_t memspace __attribute__((unused)) > + =3D (allocator_data > + ? allocator_data->memspace > + : predefined_alloc_mapping[allocator]); > + new_ptr =3D MEMSPACE_ALLOC (memspace, new_size); > if (new_ptr =3D=3D 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 =3D 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 -=3D 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=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-In-libgomp-allocator.c-omp_realloc-route-free-throug.patch" >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 --=-=-=--