Re: [PATCH] Add hypercall to mark superpages to improve performance

[Dave McCracken <dcm@mccr.org>]

On Wednesday 28 April 2010, Keir Fraser wrote:
> On 28/04/2010 15:33, "Dave McCracken" <dcm@mccr.org> wrote:
> > The current method of mapping hugepages/superpages in the hypervisor
> > involves updating the reference counts of every page in the superpage. 
> > This has proved to be a significant performance bottleneck.
> >
> > This patch adds a pair of MMUEXT hypercalls to mark and unmark a
> > superpage. Once the superpage is marked, the type is locked to writable
> > page until a companion unmark is done.  When that superpage is
> > subsequently mapped, only the first page needs to be reference counted.
> >
> > There are checks when the superpage is marked and unmarked to make sure
> > no individual page mappings have skewed the reference counts.
> 
> First of all, that changes the semantics of hugepages, since they can
> subsequently *only* be mapped as superpages. I'm not sure that's a
> restriction we want. 

That's not correct.  Individual pages can be freely mapped as writable pages 
after the superpage flag is set.  The only requirement is they all be at a base 
mapping state at the time of setting the mark, and be returned to that state 
when the mark is removed.

> Secondly, I don't really believe that the mark/unmark
> hypercalls are race-free -- bearing in mind, that other mappings (superpage
> or otherwise) can be constructed or deleted in parallel on other cpus.

I'll look into what can be done to prevent races.  I suspect some races we 
don't care about.

> Finally, does this really require new hypercalls? Could there not instead
>  be an always-enabled robust method for Xen to do superpage tracking?

I'm open to alternative suggestions on how to lock superpages into writable 
state once they're mapped without having to touch each individual page, even 
on the first map/unmap.  We could refcount superpage mappings in the base page 
of each superpage and then whenever a small page is mapped check its base 
page, but that would require an additional refcounted field in struct 
page_info.  I figured that would not be considered acceptable.

Dave McCracken