diff mbox

[malloc] Improve malloc initialization sequence

Message ID DB6PR0801MB205336037693E6A67CDC3EA6837E0@DB6PR0801MB2053.eurprd08.prod.outlook.com
State Committed
Headers

Commit Message

Wilco Dijkstra Sept. 29, 2017, 4:48 p.m. UTC 
  The current malloc initialization is quite convoluted. Instead of
sometimes calling malloc_consolidate from ptmalloc_init, call
malloc_init_state so that the main_arena is always initialized.
The special initialization can now be removed from malloc_consolidate.

GLIBC builds and tests pass, OK for commit?

ChangeLog:
2017-09-29  Wilco Dijkstra  <wdijkstr@arm.com>

        * malloc/arena.c (ptmalloc_init): Call malloc_init_state.
        * malloc/malloc.c (malloc_consolidate): Remove initialization.
--

Comments

Florian Weimer Sept. 29, 2017, 7:36 p.m. UTC | #1 
On 09/29/2017 06:48 PM, Wilco Dijkstra wrote:
>        thread attempting to use the arena in parallel waits on us till we
>        finish.  */
>     __libc_lock_lock (main_arena.mutex);
> -  malloc_consolidate (&main_arena);
> +  malloc_init_state (&main_arena);

The locking is unnecessary.  You should remove it and call 
malloc_init_state before the tunables preprocessor conditional.

I believe this fixes bug 22159, so please reference this bug (both in 
the commit message and the ChangeLog).

Thanks,
Florian
diff mbox

Patch

diff --git a/malloc/arena.c b/malloc/arena.c
index 9e5a62d260bf2f5e6d76da4ccaf7b7dcb388c296..628652c2d89ea092f7656d2ec4f3c405a39de886 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -312,7 +312,7 @@  ptmalloc_init (void)
      thread attempting to use the arena in parallel waits on us till we
      finish.  */
   __libc_lock_lock (main_arena.mutex);
-  malloc_consolidate (&main_arena);
+  malloc_init_state (&main_arena);
 
   TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
   TUNABLE_GET (top_pad, size_t, TUNABLE_CALLBACK (set_top_pad));
@@ -392,6 +392,7 @@  ptmalloc_init (void)
             }
         }
     }
+  malloc_init_state (&main_arena);
   if (s && s[0] != '\0' && s[0] != '0')
     __malloc_check_init ();
 #endif
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 88cfd25766eba6787faeb7195d95b73d7a4637ab..162e423e7bd18a07e4e97dc618be406d8bc9c529 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -4410,12 +4410,7 @@  static void malloc_consolidate(mstate av)
   mchunkptr       bck;
   mchunkptr       fwd;
 
-  /*
-    If max_fast is 0, we know that av hasn't
-    yet been initialized, in which case do so below
-  */
-
-  if (get_max_fast () != 0) {
+  {
     atomic_store_relaxed (&av->have_fastchunks, false);
 
     unsorted_bin = unsorted_chunks(av);
@@ -4484,10 +4479,6 @@  static void malloc_consolidate(mstate av)
       }
     } while (fb++ != maxfb);
   }
-  else {
-    malloc_init_state(av);
-    check_malloc_state(av);
-  }
 }
 
 /*