[updated] malloc per-thread cache ready for review

  > ChangeLog?

Included.

> > +    tcache_unsorted_limit {
> > +      type: SIZE_T
> > +      security_level: SXID_IGNORE
> > +    }
> 
> Why does this need to be inherited by children of setxid processes?  I
> guess the fact that the remaining malloc tunables are is one probable
> reason, but those also need to be reviewed and brought down to
> SXID_ERASE if necessary.

IIRC Florian asked for that setting; I don't know specifically what the
difference is but if you two want to fight it out, I'll get some popcorn ;-)

> This needs to use the new TUNABLE_CALLBACK_FNDECL.

Fixed.  My git repo has been munging my merges, it kept reverting that
trunk DL_* patch :-(

> This needs to use the new TUNABLE_GET macro.

Likewise.

> > +/* We overlay this structure on the user-data portion of a chunk when
> > +   the chunk is stored in the per-thread cache.  */
> > +typedef struct tcache_entry {
> 
> Incorrect formatting.

You weren't specific here, so I had to guess which formatting you were
referring to, as the style I used matches code elsewhere in malloc...

> > +static void __attribute__ ((section ("__libc_thread_freeres_fn")))
> 
> __attribute__ should be on its own line.

Likewise, I was copying the formatting used elsewhere in malloc.  What
is the correct formatting for that line?  Do I need to fix the other
existing instances also?

> > +  for (i = 0; i < TCACHE_MAX_BINS; ++i) {
> > +    while (tcache_tmp->entries[i])
> > +      {
> > +	tcache_entry *e = tcache_tmp->entries[i];
> > +	tcache_tmp->entries[i] = e->next;
> > +	__libc_free (e);
> > +      }
> 
> Incorrect formatting.

Not sure what formatting you're referring to here, unless it's the tabs
being messed up by the "> >"...

> Avoid implicit bool conversion for victim.

Fixed.

> Indent 1 space.

Fixed.

> Implicit boolean conversion.

Fixed.

> Unnecessary braces.

Removed, although there are pre-existing cases of that throughout the
code... at least, when the "if" clause is multi-line.

> > +  if (value >= 0 && value <= MAX_TCACHE_SIZE)
> 
> This validation should be done in tunables by setting the minval and
> maxval values in dl-tunables.list.

I don't see how I can put MAX_TCACHE_SIZE in dl-tunables.list.  Should I
hard-code the default value, and also include the test here?  Or is
there no other other use of min/max in dl-tunables.list?

Revised patch attached.

2017-06-27  DJ Delorie  <dj@delorie.com>

	* config.make.in: Enable experimental malloc option.
	* configure.ac: Likewise.
	* configure: Regenerate.
	* malloc/Makefile: Likewise.
	* malloc/malloc.c: Add per-thread cache (tcache).
	(tcache_put): New.
	(tcache_get): New.
	(tcache_thread_freeres): New.
	(tcache_init): New.
	(__libc_malloc): Use cached chunks if available.
	(__libc_free): Initialize tcache if needed.
	(__libc_realloc): Likewise.
	(__libc_calloc): Likewise.
	(_int_malloc): Prefill tcache when appropriate.
	(_int_free): Likewise.
	(do_set_tcache_max): New.
	(do_set_tcache_count): New.
	(do_set_tcache_unsorted_limit): New.
	* manual/probes.texi: Document new probes.
	* malloc/arena.c: Add new tcache tunables.
	* elf/dl-tunables.list: Likewise.
	* manual/tunables.texi: Document them.

[updated] malloc per-thread cache ready for review

Commit Message

Comments

Patch