From patchwork Wed Nov 15 13:36:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 24253 Received: (qmail 128942 invoked by alias); 15 Nov 2017 13:36:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 128928 invoked by uid 89); 15 Nov 2017 13:36:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=94411 X-HELO: mx1.redhat.com Subject: Re: malloc: Trim unused arenas on thread exit To: Siddhesh Poyarekar , DJ Delorie Cc: libc-alpha@sourceware.org References: <1b079a83-ed26-e2eb-d91f-bf3a91aa7221@redhat.com> <531db45b-f7b7-205b-5c18-0676dbbaaae4@gotplt.org> <1be14ef7-cbe3-cec9-c912-6ed159846163@gotplt.org> <1cc80c72-dd87-0975-57f3-2b388865d5c0@redhat.com> <4d496f96-7767-c1fa-1900-4edc943cc3ad@gotplt.org> From: Florian Weimer Message-ID: <1d6bd77d-670e-b7c0-0333-f05be6a3f3e1@redhat.com> Date: Wed, 15 Nov 2017 14:36:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <4d496f96-7767-c1fa-1900-4edc943cc3ad@gotplt.org> On 11/15/2017 12:19 PM, Siddhesh Poyarekar wrote: > On Wednesday 15 November 2017 04:14 PM, Florian Weimer wrote: >> It could be related to the ordering between tcache_thread_freeres and >> arena_thread_freeres.  We should merge the two to enforce the correct >> ordering. > > I agree, it sounds like a better idea to have a single freeres for a > module anyway. Here's a patch which does just that. Tested with and without --disable-experimental-malloc. It doesn't have any consolidation/trimming changes. Thanks, Florian Reviewed-by: Siddhesh Poyarekar malloc: Use a single thread shutdown hook for tcache and arenas This modifies the code to make the ordering explicit (tcache first). 2017-11-15 Florian Weimer * malloc/malloc.c (tcache_thread_shutdown): Rename from tcache_thread_freeres. Define for USE_TCACHE and !USE_TCACHE alike. Remove freeres marker. * malloc/arena.c (arena_thread_freeres): Call tcache_thread_shutdown. diff --git a/malloc/arena.c b/malloc/arena.c index 85b985e193..4d27e17c46 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -944,6 +944,11 @@ arena_get_retry (mstate ar_ptr, size_t bytes) static void __attribute__ ((section ("__libc_thread_freeres_fn"))) arena_thread_freeres (void) { + /* Shut down the thread cache first. This could deallocate data for + the thread arena, so do this before we put the arena on the free + list. */ + tcache_thread_shutdown (); + mstate a = thread_arena; thread_arena = NULL; diff --git a/malloc/malloc.c b/malloc/malloc.c index 2999ac4d2f..79f0e9eac7 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -1869,6 +1869,9 @@ void *weak_variable (*__memalign_hook) = memalign_hook_ini; void weak_variable (*__after_morecore_hook) (void) = NULL; +/* This function is called from the arena shutdown hook, to free the + thread cache (if it exists). */ +static void tcache_thread_shutdown (void); /* ------------------ Testing support ----------------------------------*/ @@ -2938,8 +2941,8 @@ tcache_get (size_t tc_idx) return (void *) e; } -static void __attribute__ ((section ("__libc_thread_freeres_fn"))) -tcache_thread_freeres (void) +static void +tcache_thread_shutdown (void) { int i; tcache_perthread_struct *tcache_tmp = tcache; @@ -2965,7 +2968,6 @@ tcache_thread_freeres (void) __libc_free (tcache_tmp); } -text_set_element (__libc_thread_subfreeres, tcache_thread_freeres); static void tcache_init(void) @@ -3002,13 +3004,20 @@ tcache_init(void) } -#define MAYBE_INIT_TCACHE() \ +# define MAYBE_INIT_TCACHE() \ if (__glibc_unlikely (tcache == NULL)) \ tcache_init(); -#else -#define MAYBE_INIT_TCACHE() -#endif +#else /* !USE_TCACHE */ +# define MAYBE_INIT_TCACHE() + +static void +tcache_thread_shutdown (void) +{ + /* Nothing to do if there is no thread cache. */ +} + +#endif /* !USE_TCACHE */ void * __libc_malloc (size_t bytes)