Skip to content

Commit

Permalink
Eliminate false sharing in malloc due to statistic collection
Browse files Browse the repository at this point in the history
Currently stats are collected in a MAXCPU-sized array which is not
aligned and suffers enormous false-sharing. Fix the problem by
utilizing per-cpu allocation.

The counter(9) API is not used here as it is too incomplete and does
not provide a win over per-cpu zone sized for malloc stats struct. In
particular stats are being reported for each cpu separately by just
copying what is supposed to be an array element for given cpu.

This eliminates significant false-sharing during malloc-heavy tests
e.g. on Skylake. See the review for details.

Reviewed by:	markj
Approved by:	re (kib)
Differential Revision:	https://reviews.freebsd.org/D17289
  • Loading branch information
mjguzik committed Sep 23, 2018
1 parent db05ff4 commit e3c2d3a
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
35 changes: 27 additions & 8 deletions sys/kern/kern_malloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ __FBSDID("$FreeBSD$");
#include <sys/vmmeter.h>
#include <sys/proc.h>
#include <sys/sbuf.h>
#include <sys/smp.h>
#include <sys/sysctl.h>
#include <sys/time.h>
#include <sys/vmem.h>
Expand Down Expand Up @@ -173,6 +174,7 @@ struct {
* declare malloc types.
*/
static uma_zone_t mt_zone;
static uma_zone_t mt_stats_zone;

u_long vm_kmem_size;
SYSCTL_ULONG(_vm, OID_AUTO, kmem_size, CTLFLAG_RDTUN, &vm_kmem_size, 0,
Expand Down Expand Up @@ -368,7 +370,7 @@ malloc_type_zone_allocated(struct malloc_type *mtp, unsigned long size,

critical_enter();
mtip = mtp->ks_handle;
mtsp = &mtip->mti_stats[curcpu];
mtsp = zpcpu_get(mtip->mti_stats);
if (size > 0) {
mtsp->mts_memalloced += size;
mtsp->mts_numallocs++;
Expand Down Expand Up @@ -411,7 +413,7 @@ malloc_type_freed(struct malloc_type *mtp, unsigned long size)

critical_enter();
mtip = mtp->ks_handle;
mtsp = &mtip->mti_stats[curcpu];
mtsp = zpcpu_get(mtip->mti_stats);
mtsp->mts_memfreed += size;
mtsp->mts_numfrees++;

Expand Down Expand Up @@ -953,6 +955,9 @@ mallocinit(void *dummy)
if (kmem_zmax < PAGE_SIZE || kmem_zmax > KMEM_ZMAX)
kmem_zmax = KMEM_ZMAX;

mt_stats_zone = uma_zcreate("mt_stats_zone",
sizeof(struct malloc_type_stats), NULL, NULL, NULL, NULL,
UMA_ALIGN_PTR, UMA_ZONE_PCPU);
mt_zone = uma_zcreate("mt_zone", sizeof(struct malloc_type_internal),
#ifdef INVARIANTS
mtrash_ctor, mtrash_dtor, mtrash_init, mtrash_fini,
Expand Down Expand Up @@ -995,6 +1000,7 @@ malloc_init(void *data)
panic("malloc_init: bad malloc type magic");

mtip = uma_zalloc(mt_zone, M_WAITOK | M_ZERO);
mtip->mti_stats = uma_zalloc_pcpu(mt_stats_zone, M_WAITOK | M_ZERO);
mtp->ks_handle = mtip;
mtp_set_subzone(mtp);

Expand Down Expand Up @@ -1042,8 +1048,8 @@ malloc_uninit(void *data)
* Look for memory leaks.
*/
temp_allocs = temp_bytes = 0;
for (i = 0; i < MAXCPU; i++) {
mtsp = &mtip->mti_stats[i];
for (i = 0; i <= mp_maxid; i++) {
mtsp = zpcpu_get_cpu(mtip->mti_stats, i);
temp_allocs += mtsp->mts_numallocs;
temp_allocs -= mtsp->mts_numfrees;
temp_bytes += mtsp->mts_memalloced;
Expand All @@ -1056,6 +1062,7 @@ malloc_uninit(void *data)
}

slab = vtoslab((vm_offset_t) mtip & (~UMA_SLAB_MASK));
uma_zfree_pcpu(mt_stats_zone, mtip->mti_stats);
uma_zfree_arg(mt_zone, mtip, slab);
}

Expand All @@ -1077,6 +1084,7 @@ sysctl_kern_malloc_stats(SYSCTL_HANDLER_ARGS)
{
struct malloc_type_stream_header mtsh;
struct malloc_type_internal *mtip;
struct malloc_type_stats *mtsp, zeromts;
struct malloc_type_header mth;
struct malloc_type *mtp;
int error, i;
Expand All @@ -1089,6 +1097,8 @@ sysctl_kern_malloc_stats(SYSCTL_HANDLER_ARGS)
sbuf_clear_flags(&sbuf, SBUF_INCLUDENUL);
mtx_lock(&malloc_mtx);

bzero(&zeromts, sizeof(zeromts));

/*
* Insert stream header.
*/
Expand All @@ -1114,10 +1124,17 @@ sysctl_kern_malloc_stats(SYSCTL_HANDLER_ARGS)
/*
* Insert type statistics for each CPU.
*/
for (i = 0; i < MAXCPU; i++) {
(void)sbuf_bcat(&sbuf, &mtip->mti_stats[i],
sizeof(mtip->mti_stats[i]));
for (i = 0; i <= mp_maxid; i++) {
mtsp = zpcpu_get_cpu(mtip->mti_stats, i);
(void)sbuf_bcat(&sbuf, mtsp, sizeof(*mtsp));
}
/*
* Fill in the missing CPUs.
*/
for (; i < MAXCPU; i++) {
(void)sbuf_bcat(&sbuf, &zeromts, sizeof(zeromts));
}

}
mtx_unlock(&malloc_mtx);
error = sbuf_finish(&sbuf);
Expand Down Expand Up @@ -1170,6 +1187,7 @@ malloc_type_list(malloc_type_list_func_t *func, void *arg)
DB_SHOW_COMMAND(malloc, db_show_malloc)
{
struct malloc_type_internal *mtip;
struct malloc_type_internal *mtsp;
struct malloc_type *mtp;
uint64_t allocs, frees;
uint64_t alloced, freed;
Expand All @@ -1183,7 +1201,8 @@ DB_SHOW_COMMAND(malloc, db_show_malloc)
frees = 0;
alloced = 0;
freed = 0;
for (i = 0; i < MAXCPU; i++) {
for (i = 0; i <= mp_maxid; i++) {
mtsp = zpcpu_get_cpu(mtip->mti_stats, i);
allocs += mtip->mti_stats[i].mts_numallocs;
frees += mtip->mti_stats[i].mts_numfrees;
alloced += mtip->mti_stats[i].mts_memalloced;
Expand Down
2 changes: 1 addition & 1 deletion sys/sys/malloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ struct malloc_type_internal {
uint32_t mti_probes[DTMALLOC_PROBE_MAX];
/* DTrace probe ID array. */
u_char mti_zone;
struct malloc_type_stats mti_stats[MAXCPU];
struct malloc_type_stats *mti_stats;
};

/*
Expand Down

0 comments on commit e3c2d3a

Please sign in to comment.