[PATCH] Fix tcache count maximum

[DJ Delorie]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> In all cases we already check tcache->counts[tc_idx] < mp_.tcache_count,
> so there can be no overflow iff mp_.tcache_count is the maximum value of
> tcache->counts[] entries. So no checks needed.
>
> 2947   tcache_entry *e = tcache->entries[tc_idx];
> 2948   assert (tc_idx < TCACHE_MAX_BINS);
> 2949   assert (tcache->counts[tc_idx] > 0);
>
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         Always true now if counts is only positive.
>         Remove?
>
> Yes this assert is redundant since we already checked there is a valid entry
> (or just added several new entries). So this assert can never trigger, it only 
> fails if tcache_put has an overflow bug.

No, counts may be zero (shouldn't, but may).  The test is there to make
sure we don't try to remove a chunk from an empty tcache and
accidentally decrement the count to MAX_CHAR or something.  The callers
of tcache_get have their own checks to see if something is in the cache,
but there's no guarantee that they're consistent (i.e. corrupted data,
other threads, malice).

>> The manual/memory.texi needs updating because you made the
>> count twice the size, and the rough estimates for size of
>> tcache should be updated. The manual should also list the
>> actual limit being imposed.
>
> Which size do you mean?

I think he's referring to sizeof(char) vs sizeof(short) in the overhead
of the tcache array itself.  I'm not sure where (or if) this is
documented.

> +#define MAX_TCACHE_COUNT 127	/* Maximum value of counts[] entries.  */

Ok.

> -  assert (tcache->counts[tc_idx] > 0);

I don't see how this is related to the overflow issue being fixed.

> -  LIBC_PROBE (memory_tunable_tcache_count, 2, value, mp_.tcache_count);
> -  mp_.tcache_count = value;
> +  if (value <= MAX_TCACHE_COUNT)

value is a size_t so will be unsigned, test for >=0 is implied so not
required.  Ok.

>  @deftp Tunable glibc.malloc.tcache_count
>  The maximum number of chunks of each size to cache. The default is 7.
> -There is no upper limit, other than available system memory.  If set
> -to zero, the per-thread cache is effectively disabled.
> +The upper limit is 127.  If set to zero, the per-thread cache is effectively
> +disabled.

Ok.