This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/3] Add missing page rounding of a page_entry
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