Message ID | 1319176370-26071-2-git-send-email-andi@firstfloor.org |
---|---|
State | New |
Headers | show |
On Fri, Oct 21, 2011 at 7:52 AM, Andi Kleen <andi@firstfloor.org> wrote: > From: Andi Kleen <ak@linux.intel.com> > > This one place in ggc forgot to round page_entry->bytes to the > next page boundary, which lead to all the heuristics in freeing to > check for continuous memory failing. Round here too, like all other > allocators already do. The memory consumed should be the same > for MMAP because the kernel would round anyways. It may slightly > increase memory usage when malloc groups are used. > > This will also increase the hitrate on the free page list > slightly. Ok. Thanks, RIchard. > gcc/: > > 2011-10-18 Andi Kleen <ak@linux.intel.com> > > * ggc-page.c (alloc_pages): Always round up entry_size. > --- > gcc/ggc-page.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c > index 2da99db..ba88e3f 100644 > --- a/gcc/ggc-page.c > +++ b/gcc/ggc-page.c > @@ -736,6 +736,7 @@ alloc_page (unsigned order) > entry_size = num_objects * OBJECT_SIZE (order); > if (entry_size < G.pagesize) > entry_size = G.pagesize; > + entry_size = ROUND_UP (entry_size, G.pagesize); > > entry = NULL; > page = NULL; > -- > 1.7.5.4 > >
On Fri, Oct 21, 2011 at 11:42:26AM +0200, Richard Guenther wrote: > On Fri, Oct 21, 2011 at 7:52 AM, Andi Kleen <andi@firstfloor.org> wrote: > > From: Andi Kleen <ak@linux.intel.com> > > > > This one place in ggc forgot to round page_entry->bytes to the > > next page boundary, which lead to all the heuristics in freeing to > > check for continuous memory failing. Round here too, like all other > > allocators already do. The memory consumed should be the same > > for MMAP because the kernel would round anyways. It may slightly > > increase memory usage when malloc groups are used. > > > > This will also increase the hitrate on the free page list > > slightly. > > > 2011-10-18 Andi Kleen <ak@linux.intel.com> > > > > * ggc-page.c (alloc_pages): Always round up entry_size. As I said in the PR, ROUND_UP should make the previous if (entry_size < G.pagesize) entry_size = G.pagesize; completely unnecessary. Additionally, seeing what ROUND_UP does, it seems horribly expensive when the second argument is not a constant. #define ROUND_UP(x, f) (CEIL (x, f) * (f)) #define CEIL(x,y) (((x) + (y) - 1) / (y)) as G.pagesize is variable, I'm afraid the compiler has to divide and multiply (or perhaps divide and modulo), there is nothing hinting that G.pagesize is a power of two and thus (entry_page_size + G.pagesize - 1) & (G.pagesize - 1); will work. ggc-page.c relies on G.pagesize to be a power of two though (and I hope no sane host uses something else), as otherwise G.lg_pagesize would be -1 and we shift by that amount, so that would be undefined behavior. > > diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c > > index 2da99db..ba88e3f 100644 > > --- a/gcc/ggc-page.c > > +++ b/gcc/ggc-page.c > > @@ -736,6 +736,7 @@ alloc_page (unsigned order) > > entry_size = num_objects * OBJECT_SIZE (order); > > if (entry_size < G.pagesize) > > entry_size = G.pagesize; > > + entry_size = ROUND_UP (entry_size, G.pagesize); > > > > entry = NULL; > > page = NULL; > > -- > > 1.7.5.4 > > > > Jakub
On Fri, Oct 21, 2011 at 11:52:44AM +0200, Jakub Jelinek wrote: > On Fri, Oct 21, 2011 at 11:42:26AM +0200, Richard Guenther wrote: > > On Fri, Oct 21, 2011 at 7:52 AM, Andi Kleen <andi@firstfloor.org> wrote: > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > This one place in ggc forgot to round page_entry->bytes to the > > > next page boundary, which lead to all the heuristics in freeing to > > > check for continuous memory failing. Round here too, like all other > > > allocators already do. The memory consumed should be the same > > > for MMAP because the kernel would round anyways. It may slightly > > > increase memory usage when malloc groups are used. > > > > > > This will also increase the hitrate on the free page list > > > slightly. > > > > > 2011-10-18 Andi Kleen <ak@linux.intel.com> > > > > > > * ggc-page.c (alloc_pages): Always round up entry_size. > > As I said in the PR, ROUND_UP should make the previous > if (entry_size < G.pagesize) > entry_size = G.pagesize; > completely unnecessary. AFAIK there are objects > pagesize in GGC, but not too many But you're right it will be somewhat expensive (although the mmap is not very common anyways and most other allocations already do the roundup). I can drop it. -Andi
diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c index 2da99db..ba88e3f 100644 --- a/gcc/ggc-page.c +++ b/gcc/ggc-page.c @@ -736,6 +736,7 @@ alloc_page (unsigned order) entry_size = num_objects * OBJECT_SIZE (order); if (entry_size < G.pagesize) entry_size = G.pagesize; + entry_size = ROUND_UP (entry_size, G.pagesize); entry = NULL; page = NULL;
From: Andi Kleen <ak@linux.intel.com> This one place in ggc forgot to round page_entry->bytes to the next page boundary, which lead to all the heuristics in freeing to check for continuous memory failing. Round here too, like all other allocators already do. The memory consumed should be the same for MMAP because the kernel would round anyways. It may slightly increase memory usage when malloc groups are used. This will also increase the hitrate on the free page list slightly. gcc/: 2011-10-18 Andi Kleen <ak@linux.intel.com> * ggc-page.c (alloc_pages): Always round up entry_size. --- gcc/ggc-page.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)