Message ID | 20150210202703.GC21275@suse.de |
---|---|
State | New |
Headers | show |
On Tue, Feb 10, 2015 at 08:27:03PM +0000, Mel Gorman wrote: > Trimming heaps is a balance between saving memory and the system overhead > required to update page tables and discard allocated pages. The malloc > option M_TRIM_THRESHOLD is a tunable that users are meant to use to decide > where this balance point is but it is only applied to the main arena. > > For scalability reasons, glibc malloc has per-thread heaps but these are > shrunk with madvise() if there is one page free at the top of the heap. > In some circumstances this can lead to high system overhead if a thread > has a control flow like > > while (data_to_process) { > buf = malloc(large_size); > do_stuff(); > free(buf); > } > > For a large size, the free() will call madvise (pagetable teardown, page > free and TLB flush) every time followed immediately by a malloc (fault, > kernel page alloc, zeroing and charge accounting). The kernel overhead > can dominate such a workload. > > This patch allows the user to tune when madvise gets called by applying > the trim threshold to the per-thread heaps. Thanks for fixing this. I consider this a bug, since the complementary tunable MALLOC_MMAP_THRESHOLD_ does get applied to non-main arenas. > diff --git a/ChangeLog b/ChangeLog > index dc1ed1ba1249..b860b2fe1850 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,8 @@ > +2015-02-10 Mel Gorman <mgorman@suse.de> > + > + * malloc/arena.c (free): Apply trim threshold to per-thread heaps > + as well as the main arena. > + > 2015-02-06 Carlos O'Donell <carlos@systemhalted.org> This should not be in a diff since it won't always apply to the current tree cleanly. Please file a bug for this (since it is an unintended inconsistency) and I'll push it for you. Thanks, Siddhesh
On Wed, Feb 18, 2015 at 10:58:03AM +0530, Siddhesh Poyarekar wrote: > This should not be in a diff since it won't always apply to the > current tree cleanly. Please file a bug for this (since it is an > unintended inconsistency) and I'll push it for you. And I just noticed that you posted a v3 with a quoted bug report. The only additional nit there is that the ChangeLog entry should mention the BZ #. I'll push this. Siddhesh
On Wed, Feb 18, 2015 at 11:19:49AM +0530, Siddhesh Poyarekar wrote: > On Wed, Feb 18, 2015 at 10:58:03AM +0530, Siddhesh Poyarekar wrote: > > This should not be in a diff since it won't always apply to the > > current tree cleanly. Please file a bug for this (since it is an > > unintended inconsistency) and I'll push it for you. > > And I just noticed that you posted a v3 with a quoted bug report. The > only additional nit there is that the ChangeLog entry should mention > the BZ #. I'll push this. > Thanks for pointing out the hazards with the patch. The Changelog was included in the diff because it was done manually because I though that was required. I don't think it needs another bug to fix up as it is a misunderstanding on my part. There will be a v4 shortly that hopefully confirms to the patch rules.
diff --git a/ChangeLog b/ChangeLog index dc1ed1ba1249..b860b2fe1850 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-02-10 Mel Gorman <mgorman@suse.de> + + * malloc/arena.c (free): Apply trim threshold to per-thread heaps + as well as the main arena. + 2015-02-06 Carlos O'Donell <carlos@systemhalted.org> * version.h (RELEASE): Set to "stable". diff --git a/malloc/arena.c b/malloc/arena.c index 886defb074a2..a78d4835a825 100644 --- a/malloc/arena.c +++ b/malloc/arena.c @@ -696,7 +696,7 @@ heap_trim (heap_info *heap, size_t pad) } top_size = chunksize (top_chunk); extra = (top_size - pad - MINSIZE - 1) & ~(pagesz - 1); - if (extra < (long) pagesz) + if (extra < (long) mp_.trim_threshold) return 0; /* Try to shrink. */