[patch v3] malloc: avoid need for tcache == NULL checks

[Florian Weimer]

* DJ Delorie:

> +/* TCACHE is never NULL; it's either "live" or points to one of the
> +   above dummy entries.  The dummy entries are all zero so act like an
> +   empty/unusable tcache.  */
> +static __thread tcache_perthread_struct *tcache = (tcache_perthread_struct *) &__tcache_dummy.inactive;

Line is too long.

>  /* Initialize tcache.  In the rare case there isn't any memory available,
>     later calls will retry initialization.  */
>  static void
> -tcache_init (void)
> +tcache_init (mstate av)
>  {
> -  if (tcache_shutting_down)
> +  /* Check minimum mmap chunk is larger than max tcache size.  This means
> +     mmap chunks with their different layout are never added to tcache.  */
> +  if (MAX_TCACHE_SMALL_SIZE >= GLRO (dl_pagesize) / 2)
> +    malloc_printerr ("max tcache size too large");

Uhm, this was removed in:

commit 614cfd0f8a2820aed54f9745077c7da0e6643bac
Author: Wilco Dijkstra <wilco.dijkstra@arm.com>
Date:   Fri Aug 8 14:11:13 2025 +0000

    malloc: Change mmap chunk layout
    
    Change the mmap chunk layout to be identical to a normal chunk.  This makes it
    safe for tcache to hold mmap chunks and simplifies size calculations in
    memsize and musable.  Add mmap_base() and mmap_size() macros to simplify code.
    
    Reviewed-by: Cupertino Miranda <cupertino.miranda@oracle.com>


> @@ -3510,8 +3550,11 @@ __libc_free (void *mem)
>      return malloc_printerr_tail ("free(): invalid pointer");
>  
>  #if USE_TCACHE
> -  if (__glibc_likely (size < mp_.tcache_max_bytes && tcache != NULL))
> +  if (__glibc_likely (size < mp_.tcache_max_bytes))
>      {
> +      if (__glibc_unlikely (tcache_inactive ()))
> +	tcache_init (NULL);
> +
>        /* Check to see if it's already in the tcache.  */
>        tcache_entry *e = (tcache_entry *) chunk2mem (p);

This should come after we checked if there was room in tcache and found
none.  And I hope that


  if (__glibc_unlikely (tcache_inactive ()))
    {
      tcache_init (NULL);
      return __libc_free (mem);
    }

generates good code.  We don't want GCC to put NOPs into the middle of
free, to align a loop that pretty much always is only exected once.

> @@ -3896,9 +3939,6 @@ __libc_calloc (size_t n, size_t elem_size)
>  
>    if (nb < mp_.tcache_max_bytes)
>      {
> -      if (__glibc_unlikely (tcache == NULL))
> -	return tcache_calloc_init (bytes);
> -
>        size_t tc_idx = csize2tidx (nb);
>  
>        if (__glibc_unlikely (tc_idx < TCACHE_SMALL_BINS))
> @@ -4030,10 +4070,14 @@ _int_malloc (mstate av, size_t bytes)
>  	      /* While we're here, if we see other chunks of the same size,
>  		 stash them in the tcache.  */
>  	      size_t tc_idx = csize2tidx (nb);
> -	      if (tcache != NULL && tc_idx < mp_.tcache_small_bins)
> +	      if (tc_idx < mp_.tcache_small_bins)
>  		{
>  		  mchunkptr tc_victim;
>  
> +#if USE_TCACHE
> +		  if (__glibc_unlikely (tcache_inactive ()))
> +		    tcache_init (av);
> +#endif
>  		  /* While bin not empty and tcache not full, copy chunks.  */
>  		  while (tcache->num_slots[tc_idx] != 0 && (tc_victim = *fb) != NULL)
>  		    {

I don't think you can call into malloc at this point because it may
invalidate the bin contents.  So the while loop should be in the else
part of the if (with a comment that tcache_init sets up the cache for
future calls).

> @@ -4090,10 +4134,14 @@ _int_malloc (mstate av, size_t bytes)
>  	  /* While we're here, if we see other chunks of the same size,
>  	     stash them in the tcache.  */
>  	  size_t tc_idx = csize2tidx (nb);
> -	  if (tcache != NULL && tc_idx < mp_.tcache_small_bins)
> +	  if (tc_idx < mp_.tcache_small_bins)
>  	    {
>  	      mchunkptr tc_victim;
>  
> +#if USE_TCACHE
> +	      if (__glibc_unlikely (tcache_inactive ()))
> +		tcache_init (av);
> +#endif
>  	      /* While bin not empty and tcache not full, copy chunks over.  */
>  	      while (tcache->num_slots[tc_idx] != 0
>  		     && (tc_victim = last (bin)) != bin)

Likely same problem here.

> @@ -4152,7 +4200,7 @@ _int_malloc (mstate av, size_t bytes)
>  #if USE_TCACHE
>    INTERNAL_SIZE_T tcache_nb = 0;
>    size_t tc_idx = csize2tidx (nb);
> -  if (tcache != NULL && tc_idx < mp_.tcache_small_bins)
> +  if (tc_idx < mp_.tcache_small_bins)
>      tcache_nb = nb;
>    int return_cached = 0;
>  
> @@ -4230,6 +4278,8 @@ _int_malloc (mstate av, size_t bytes)
>                if (av != &main_arena)
>  		set_non_main_arena (victim);
>  #if USE_TCACHE
> +	      if (__glibc_unlikely (tcache_inactive ()))
> +		tcache_init (av);
>  	      /* Fill cache first, return to user only if cache fills.
>  		 We may return one of these chunks later.  */
>  	      if (tcache_nb > 0

And here.  The tcache_init call should probably on the else branch that
returns p below.

Thanks,
Florian