Re: malloc: Trim unused arenas on thread exit

[Florian Weimer <fweimer at redhat dot com>]

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

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  <fweimer@redhat.com>

	* 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)