[PATCH] malloc: unifdef -m -UATOMIC_FASTBINS

[Joern Engel <joern at purestorage dot com>]

From: Joern Engel <joern@purestorage.org>

Atomic fastbins are a horrible eye-sore, known to be buggy in our
version and cause unnecessary cacheline pingpong.  We can do better than
that.

JIRA: PURE-27597
---
 tpc/malloc2.13/arena.h  |   6 --
 tpc/malloc2.13/hooks.h  |   8 ---
 tpc/malloc2.13/malloc.c | 148 ------------------------------------------------
 3 files changed, 162 deletions(-)

diff --git a/tpc/malloc2.13/arena.h b/tpc/malloc2.13/arena.h
index 6ac0635364af..ef5e22a0811d 100644
--- a/tpc/malloc2.13/arena.h
+++ b/tpc/malloc2.13/arena.h
@@ -169,11 +169,6 @@ free_atfork(Void_t* mem, const Void_t *caller)
     return;
   }
 
-#ifdef ATOMIC_FASTBINS
-  ar_ptr = arena_for_chunk(p);
-  tsd_getspecific(arena_key, vptr);
-  _int_free(ar_ptr, p, vptr == ATFORK_ARENA_PTR);
-#else
   ar_ptr = arena_for_chunk(p);
   tsd_getspecific(arena_key, vptr);
   if(vptr != ATFORK_ARENA_PTR)
@@ -181,7 +176,6 @@ free_atfork(Void_t* mem, const Void_t *caller)
   _int_free(ar_ptr, p);
   if(vptr != ATFORK_ARENA_PTR)
     (void)mutex_unlock(&ar_ptr->mutex);
-#endif
 }
 
 
diff --git a/tpc/malloc2.13/hooks.h b/tpc/malloc2.13/hooks.h
index c236aab89237..f855b0f1b165 100644
--- a/tpc/malloc2.13/hooks.h
+++ b/tpc/malloc2.13/hooks.h
@@ -257,11 +257,7 @@ free_check(Void_t* mem, const Void_t *caller)
 #if 0 /* Erase freed memory. */
   memset(mem, 0, chunksize(p) - (SIZE_SZ+1));
 #endif
-#ifdef ATOMIC_FASTBINS
-  _int_free(&main_arena, p, 1);
-#else
   _int_free(&main_arena, p);
-#endif
   (void)mutex_unlock(&main_arena.mutex);
 }
 
@@ -406,11 +402,7 @@ free_starter(Void_t* mem, const Void_t *caller)
     munmap_chunk(p);
     return;
   }
-#ifdef ATOMIC_FASTBINS
-  _int_free(&main_arena, p, 1);
-#else
   _int_free(&main_arena, p);
-#endif
 }
 
 # endif	/* !defiend NO_STARTER */
diff --git a/tpc/malloc2.13/malloc.c b/tpc/malloc2.13/malloc.c
index 9df824584745..ddc9d51c445b 100644
--- a/tpc/malloc2.13/malloc.c
+++ b/tpc/malloc2.13/malloc.c
@@ -1358,11 +1358,7 @@ struct mallinfo2 {
 
 
 static Void_t*  _int_malloc(struct malloc_state *, size_t);
-#ifdef ATOMIC_FASTBINS
-static void     _int_free(struct malloc_state *, mchunkptr, int);
-#else
 static void     _int_free(struct malloc_state *, mchunkptr);
-#endif
 static Void_t*  _int_realloc(struct malloc_state *, mchunkptr, INTERNAL_SIZE_T,
 			     INTERNAL_SIZE_T);
 static Void_t*  _int_memalign(struct malloc_state *, size_t, size_t);
@@ -2035,13 +2031,8 @@ typedef struct malloc_chunk* mfastbinptr;
 #define FASTCHUNKS_BIT        (1U)
 
 #define have_fastchunks(M)     (((M)->flags &  FASTCHUNKS_BIT) == 0)
-#ifdef ATOMIC_FASTBINS
-#define clear_fastchunks(M)    catomic_or (&(M)->flags, FASTCHUNKS_BIT)
-#define set_fastchunks(M)      catomic_and (&(M)->flags, ~FASTCHUNKS_BIT)
-#else
 #define clear_fastchunks(M)    ((M)->flags |=  FASTCHUNKS_BIT)
 #define set_fastchunks(M)      ((M)->flags &= ~FASTCHUNKS_BIT)
-#endif
 
 /*
   NONCONTIGUOUS_BIT indicates that MORECORE does not return contiguous
@@ -2756,10 +2747,8 @@ static Void_t* sYSMALLOc(INTERNAL_SIZE_T nb, struct malloc_state * av)
   /* Precondition: not enough current space to satisfy nb request */
   assert((unsigned long)(old_size) < (unsigned long)(nb + MINSIZE));
 
-#ifndef ATOMIC_FASTBINS
   /* Precondition: all fastbins are consolidated */
   assert(!have_fastchunks(av));
-#endif
 
 
   if (av != &main_arena) {
@@ -2805,11 +2794,7 @@ static Void_t* sYSMALLOc(INTERNAL_SIZE_T nb, struct malloc_state * av)
 	set_head(chunk_at_offset(old_top, old_size), (2*SIZE_SZ)|PREV_INUSE);
 	set_foot(chunk_at_offset(old_top, old_size), (2*SIZE_SZ));
 	set_head(old_top, old_size|PREV_INUSE|NON_MAIN_ARENA);
-#ifdef ATOMIC_FASTBINS
-	_int_free(av, old_top, 1);
-#else
 	_int_free(av, old_top);
-#endif
       } else {
 	set_head(old_top, (old_size + 2*SIZE_SZ)|PREV_INUSE);
 	set_foot(old_top, (old_size + 2*SIZE_SZ));
@@ -3049,11 +3034,7 @@ static Void_t* sYSMALLOc(INTERNAL_SIZE_T nb, struct malloc_state * av)
 
 	  /* If possible, release the rest. */
 	  if (old_size >= MINSIZE) {
-#ifdef ATOMIC_FASTBINS
-	    _int_free(av, old_top, 1);
-#else
 	    _int_free(av, old_top);
-#endif
 	  }
 
 	}
@@ -3314,13 +3295,9 @@ public_fREe(Void_t* mem)
   }
 
   ar_ptr = arena_for_chunk(p);
-#ifdef ATOMIC_FASTBINS
-  _int_free(ar_ptr, p, 0);
-#else
   arena_lock(ar_ptr);
   _int_free(ar_ptr, p);
   arena_unlock(ar_ptr);
-#endif
 }
 
 Void_t*
@@ -3400,13 +3377,9 @@ public_rEALLOc(Void_t* oldmem, size_t bytes)
       if (newp != NULL)
 	{
 	  MALLOC_COPY (newp, oldmem, oldsize - SIZE_SZ);
-#ifdef ATOMIC_FASTBINS
-	  _int_free(ar_ptr, oldp, 0);
-#else
 	  arena_lock(ar_ptr);
 	  _int_free(ar_ptr, oldp);
 	  arena_unlock(ar_ptr);
-#endif
 	}
     }
 
@@ -3771,19 +3744,7 @@ _int_malloc(struct malloc_state * av, size_t bytes)
   if ((unsigned long)(nb) <= (unsigned long)(get_max_fast ())) {
     idx = fastbin_index(nb);
     mfastbinptr* fb = &fastbin (av, idx);
-#ifdef ATOMIC_FASTBINS
-    mchunkptr pp = *fb;
-    do
-      {
-	victim = pp;
-	if (victim == NULL)
-	  break;
-      }
-    while ((pp = catomic_compare_and_exchange_val_acq (fb, victim->fd, victim))
-	   != victim);
-#else
     victim = *fb;
-#endif
     if (victim != 0) {
       if (fastbin_index (chunksize (victim)) != idx)
 	{
@@ -3792,9 +3753,7 @@ _int_malloc(struct malloc_state * av, size_t bytes)
 	  malloc_printerr (check_action, errstr, chunk2mem (victim));
 	  return NULL;
 	}
-#ifndef ATOMIC_FASTBINS
       *fb = victim->fd;
-#endif
       check_remalloced_chunk(av, victim, nb);
       void *p = chunk2mem(victim);
       if (perturb_byte)
@@ -4199,18 +4158,6 @@ _int_malloc(struct malloc_state * av, size_t bytes)
       return p;
     }
 
-#ifdef ATOMIC_FASTBINS
-    /* When we are using atomic ops to free fast chunks we can get
-       here for all block sizes.  */
-    else if (have_fastchunks(av)) {
-      malloc_consolidate(av);
-      /* restore original bin index */
-      if (in_smallbin_range(nb))
-	idx = smallbin_index(nb);
-      else
-	idx = largebin_index(nb);
-    }
-#else
     /*
       If there is space available in fastbins, consolidate and retry,
       to possibly avoid expanding memory. This can occur only if nb is
@@ -4222,7 +4169,6 @@ _int_malloc(struct malloc_state * av, size_t bytes)
       malloc_consolidate(av);
       idx = smallbin_index(nb); /* restore original bin index */
     }
-#endif
 
     /*
        Otherwise, relay to handle system-dependent cases
@@ -4241,11 +4187,7 @@ _int_malloc(struct malloc_state * av, size_t bytes)
 */
 
 static void
-#ifdef ATOMIC_FASTBINS
-_int_free(struct malloc_state * av, mchunkptr p, int have_lock)
-#else
 _int_free(struct malloc_state * av, mchunkptr p)
-#endif
 {
   INTERNAL_SIZE_T size;        /* its size */
   mfastbinptr*    fb;          /* associated fastbin */
@@ -4257,9 +4199,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
   mchunkptr       fwd;         /* misc temp for linking */
 
   const char *errstr = NULL;
-#ifdef ATOMIC_FASTBINS
-  int locked = 0;
-#endif
 
   size = chunksize(p);
 
@@ -4272,10 +4211,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
     {
       errstr = "free(): invalid pointer";
     errout:
-#ifdef ATOMIC_FASTBINS
-      if (! have_lock && locked)
-	arena_unlock(av);
-#endif
       malloc_printerr (check_action, errstr, chunk2mem(p));
       return;
     }
@@ -4308,29 +4243,10 @@ _int_free(struct malloc_state * av, mchunkptr p)
 	|| chunksize (chunk_at_offset (p, size))
 			     >= av->system_mem)
       {
-#ifdef ATOMIC_FASTBINS
-	/* We might not have a lock at this point and concurrent modifications
-	   of system_mem might have let to a false positive.  Redo the test
-	   after getting the lock.  */
-	if (have_lock
-	    || ({ assert (locked == 0);
-		  arena_lock*av);
-		  locked = 1;
-		  chunk_at_offset (p, size)->size <= 2 * SIZE_SZ
-		    || chunksize (chunk_at_offset (p, size)) >= av->system_mem;
-	      }))
-#endif
 	  {
 	    errstr = "free(): invalid next size (fast)";
 	    goto errout;
 	  }
-#ifdef ATOMIC_FASTBINS
-	if (! have_lock)
-	  {
-	    arena_unlock(av);
-	    locked = 0;
-	  }
-#endif
       }
 
     if (perturb_byte)
@@ -4340,31 +4256,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
     unsigned int idx = fastbin_index(size);
     fb = &fastbin (av, idx);
 
-#ifdef ATOMIC_FASTBINS
-    mchunkptr fd;
-    mchunkptr old = *fb;
-    unsigned int old_idx = ~0u;
-    do
-      {
-	/* Another simple check: make sure the top of the bin is not the
-	   record we are going to add (i.e., double free).  */
-	if (old == p)
-	  {
-	    errstr = "double free or corruption (fasttop)";
-	    goto errout;
-	  }
-	if (old != NULL)
-	  old_idx = fastbin_index(chunksize(old));
-	p->fd = fd = old;
-      }
-    while ((old = catomic_compare_and_exchange_val_rel (fb, p, fd)) != fd);
-
-    if (fd != NULL && old_idx != idx)
-      {
-	errstr = "invalid fastbin entry (free)";
-	goto errout;
-      }
-#else
     /* Another simple check: make sure the top of the bin is not the
        record we are going to add (i.e., double free).  */
     if (*fb == p)
@@ -4381,7 +4272,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
 
     p->fd = *fb;
     *fb = p;
-#endif
   }
 
   /*
@@ -4389,12 +4279,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
   */
 
   else if (!chunk_is_mmapped(p)) {
-#ifdef ATOMIC_FASTBINS
-    if (! have_lock) {
-      arena_lock(av);
-      locked = 1;
-    }
-#endif
 
     nextchunk = chunk_at_offset(p, size);
 
@@ -4524,12 +4408,6 @@ _int_free(struct malloc_state * av, mchunkptr p)
       }
     }
 
-#ifdef ATOMIC_FASTBINS
-    if (! have_lock) {
-      assert (locked);
-      arena_unlock(av);
-    }
-#endif
   }
   /*
     If the chunk was allocated via mmap, release via munmap(). Note
@@ -4605,15 +4483,9 @@ static void malloc_consolidate(struct malloc_state * av)
 #endif
     fb = &fastbin (av, 0);
     do {
-#ifdef ATOMIC_FASTBINS
-      p = atomic_exchange_acq (fb, 0);
-#else
       p = *fb;
-#endif
       if (p != 0) {
-#ifndef ATOMIC_FASTBINS
 	*fb = 0;
-#endif
 	do {
 	  check_inuse_chunk(av, p);
 	  nextp = p->fd;
@@ -4804,11 +4676,7 @@ _int_realloc(struct malloc_state * av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 	    }
 	  }
 
-#ifdef ATOMIC_FASTBINS
-	  _int_free(av, oldp, 1);
-#else
 	  _int_free(av, oldp);
-#endif
 	  check_inuse_chunk(av, newp);
 	  return chunk2mem(newp);
 	}
@@ -4832,11 +4700,7 @@ _int_realloc(struct malloc_state * av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 	       (av != &main_arena ? NON_MAIN_ARENA : 0));
       /* Mark remainder as inuse so free() won't complain */
       set_inuse_bit_at_offset(remainder, remainder_size);
-#ifdef ATOMIC_FASTBINS
-      _int_free(av, remainder, 1);
-#else
       _int_free(av, remainder);
-#endif
     }
 
     check_inuse_chunk(av, newp);
@@ -4895,11 +4759,7 @@ _int_realloc(struct malloc_state * av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
       newmem = _int_malloc(av, nb - MALLOC_ALIGN_MASK);
       if (newmem != 0) {
 	MALLOC_COPY(newmem, chunk2mem(oldp), oldsize - 2*SIZE_SZ);
-#ifdef ATOMIC_FASTBINS
-	_int_free(av, oldp, 1);
-#else
 	_int_free(av, oldp);
-#endif
       }
     }
     return newmem;
@@ -4988,11 +4848,7 @@ _int_memalign(struct malloc_state * av, size_t alignment, size_t bytes)
 	     (av != &main_arena ? NON_MAIN_ARENA : 0));
     set_inuse_bit_at_offset(newp, newsize);
     set_head_size(p, leadsize | (av != &main_arena ? NON_MAIN_ARENA : 0));
-#ifdef ATOMIC_FASTBINS
-    _int_free(av, p, 1);
-#else
     _int_free(av, p);
-#endif
     p = newp;
 
     assert (newsize >= nb &&
@@ -5008,11 +4864,7 @@ _int_memalign(struct malloc_state * av, size_t alignment, size_t bytes)
       set_head(remainder, remainder_size | PREV_INUSE |
 	       (av != &main_arena ? NON_MAIN_ARENA : 0));
       set_head_size(p, nb);
-#ifdef ATOMIC_FASTBINS
-      _int_free(av, remainder, 1);
-#else
       _int_free(av, remainder);
-#endif
     }
   }
 
-- 
2.7.0.rc3