Add hypercall to mark superpages to improve performance
PATCH 1/1 Add hypercall to mark superpages to improve performance
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.
Dave McCracken
Oracle Corp
/xen/include/public/xen.h +9 -1
@@ -250,6 +250,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);* cmd: MMUEXT_COPY_PAGE * mfn: Machine frame number of the destination page. * src_mfn: Machine frame number of the source page. +* +* cmd: MMUEXT_MARK_SUPER +* mfn: Machine frame number of head of superpage to be marked. +* +* cmd: MMUEXT_UNMARK_SUPER +* mfn: Machine frame number of head of superpage to be cleared. */ #define MMUEXT_PIN_L1_TABLE 0#define MMUEXT_PIN_L2_TABLE 1
@@ -268,13 +274,15 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);#define MMUEXT_NEW_USER_BASEPTR 15#define MMUEXT_CLEAR_PAGE 16#define MMUEXT_COPY_PAGE 17 +#define MMUEXT_MARK_SUPER 18 +#define MMUEXT_UNMARK_SUPER 19 #ifndef __ASSEMBLY__ struct mmuext_op { unsigned int cmd; union { /* [UN]PIN_TABLE, NEW_BASEPTR, NEW_USER_BASEPTR - * CLEAR_PAGE, COPY_PAGE */ + * CLEAR_PAGE, COPY_PAGE, [UN]MARK_SUPER */ xen_pfn_t mfn; /* INVLPG_LOCAL, INVLPG_ALL, SET_LDT */ unsigned long linear_addr;
/xen/include/asm-x86/mm.h +4 -1
@@ -182,9 +182,12 @@ struct page_info/* Page is locked? */ #define _PGT_locked PG_shift(9)#define PGT_locked PG_mask(1, 9) +/* Page is part of a superpage? */ +#define _PGT_super PG_shift(10) +#define PGT_super PG_mask(1, 10)/* Count of uses of this frame as its current type. */ -#define PGT_count_width PG_shift(9) +#define PGT_count_width PG_shift(10)#define PGT_count_mask ((1UL<<PGT_count_width)-1)/* Cleared when the owning guest 'frees' this page. */
/xen/arch/x86/domain.c +11 -0
@@ -1748,6 +1748,17 @@ static int relinquish_memory(BUG (); } +if (test_and_clear_bit (_PGT_super, &page->u.inuse.type_info) ) + { +unsigned long mfn =page_to_mfn (page); +int i; + +for (i =0 ; i <L1_PAGETABLE_ENTRIES ; i++, mfn++) + { +put_page_and_type (mfn_to_page (mfn)); + } + } +if (test_and_clear_bit (_PGC_allocated, &page->count_info) )put_page (page);
/xen/arch/x86/mm.c +124 -16
163 lines not on this page · raw message
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 11 lines · Dave McCracken
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. 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. Finally, does this really require new hypercalls? Could there not instead be an always-enabled robust method for Xen to do superpage tracking?
-- Keir
Quoted 10 lines · Keir Fraser
Quoted 9 lines · Dave McCracken
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.
Quoted 6 lines
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 2 lines
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
Quoted 6 lines · Dave McCracken
Quoted 3 lines
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.
I mean I think there are probably races that can result in inconsistent reference counts. I reckon I'll be able to find some when I'm back from travelling next week. Obviously, any such race would be unacceptable.
-- Keir
Quoted 10 lines · Keir Fraser
Quoted 6 lines
Quoted 3 lines
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.
I mean I think there are probably races that can result in inconsistent reference counts. I reckon I'll be able to find some when I'm back from travelling next week. Obviously, any such race would be unacceptable.
Here's one that's not even really a race: what if a guest marks a superpage (via hypercall), then creates a mapping of that superpage, then creates single-page mappings of all pages in the superpage except the first, then unmarks the superpage? All refcounts of pages in the superpage will be the same (because the first page holds the superpage mapping count, and all other pages have a count from their respectibve single-page mappings). So unmark would succeed, but now destroying the superpage mapping will erroneously decrement the refcount of every page in the superpage?
-- Keir
Quoted 9 lines · Dave McCracken
Quoted 2 lines
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.
One option would be an array of reference counts indexed by superpage number (i.e, mfn>>9). So kind of a separate array to page_info, and a non-zero superpage refcount would arrange to hold a reference on every relevant page in page_info.
That could be implemented with no extra hypercalls, and I reckon it's probably easier to make this race-free too. Obviously it does have extra code complexity to construct this array (which I suppose needs to be sparse, just like page_info array, in the face of very sparse memory maps). The space overhead (about 8 bytes per 2MB, or 0.0004% of total system memory) would be trivial. Compared with an extra reference count in every page_info, which would have a much higher 0.2% overhead.
-- Keir
Quoted 5 lines · Keir Fraser
One option would be an array of reference counts indexed by superpage
number (i.e, mfn>>9). So kind of a separate array topage_info, and a non-zero superpage refcount would arrange to hold a reference on every relevant page inpage_info.
Quoted 7 lines
That could be implemented with no extra hypercalls, and I reckon it's probably easier to make this race-free too. Obviously it does have extra code complexity to construct this array (which I suppose needs to be
sparse, just likepage_infoarray, in the face of very sparse memory maps). The space overhead (about 8 bytes per 2MB, or 0.0004% of total system memory) would be trivial. Compared with an extra reference count in everypage_info, which would have a much higher 0.2% overhead.
I like this idea. I'll look into it.
Dave
Quoted 9 lines · Dave McCracken
Quoted 7 lines
That could be implemented with no extra hypercalls, and I reckon it's probably easier to make this race-free too. Obviously it does have extra code complexity to construct this array (which I suppose needs to be
sparse, just likepage_infoarray, in the face of very sparse memory maps). The space overhead (about 8 bytes per 2MB, or 0.0004% of total system memory) would be trivial. Compared with an extra reference count in everypage_info, which would have a much higher 0.2% overhead.I like this idea. I'll look into it.
The algorithm for acquiring a superpage refcount would be something like:
y = superpage_info->count
do {
x = y
if ( x == 0 )
for (each page in super_page)
if (!get_page(page))
goto undo_and_fail; } while ((y = cmpxchg(&superpage_info->count, x, x+1)) != x);
For destroying a superpage refcount:
y = superpage_info->count
do { x = y } while ((y = cmpxchg(..,x,x-1)) != x); if (x==1) for (each page in super_page) put_page(page)
I'd actually have two refcounts in superpage struct: one for read-only mappings and one for read-write mappings. The latter would be updated as above except for the use of {get,put}_page_and_type() instead of {get,put}_page().
The other thing to note is that this approach is cheap when updating superpage refcounts between non-zero values. If regularly constructing/destroying the *only* superpage mapping of a page, then obviously you are going to be continually taking the slow path. In that case, pinning a superpage with new hypercalls as you suggested may have to be done. It kind of depends on how your workloads interact with the reference-counting. In any case, you could implement the basic version as described here, and add hypercalls as a second stage if they turn out to be needed.
I can help with further details and advice if need be.
-- Keir
Quoted 5 lines · Keir Fraser
I'd actually have two refcounts in superpage struct: one for read-only mappings and one for read-write mappings. The latter would be updated as above except for the use of {get,put}_page_and_type() instead of {get,put}_page().
Quoted 9 lines
The other thing to note is that this approach is cheap when updating superpage refcounts between non-zero values. If regularly constructing/destroying the only superpage mapping of a page, then obviously you are going to be continually taking the slow path. In that case, pinning a superpage with new hypercalls as you suggested may have to be done. It kind of depends on how your workloads interact with the reference-counting. In any case, you could implement the basic version as described here, and add hypercalls as a second stage if they turn out to be needed.
I have an alternative idea...
Make the superpage struct contain a single typecount field. The two possible values fortype would be "superpage" and "conflicts with superpage". The semantics of this typecount field would be the same as page_info... it can only change type when the count is zero.
The "superpage" type counts the times this superpage is mapped.
The "conflicts with superpage" type is used whenever any page in the superpage is set to a type that would conflict with a superpage mapping, basically any type other than readonly or read/write. The count would be the sum of the mappings of the pages in the superpage with a conflicting type.
Permission checking would be very simple. If get_superpage_and_type() returns a type mismatch error then the mapping will fail, in exactly the same fashion as get_page_and_type().
This model would completely eliminate the need to walk the pages in a superpage at mapping time, at the cost of one get_superpage_and_type()/put_superpage_and_type(). on each page table page.
One outstanding issue I see is how to handle readonly mappings. If we follow the model of regular page typecount, readonly mappings of superpages would not conflict with the "conflicts with superpage" type. This means a subsequent attempt to change it to a read/write mapping could fail, just like with a regular page. Or we could count all mappings of superpages as if they were read/write.
What are your thoughts? It seems fairly simple and elegant to me, and at this point I don't see any big holes in it.
Dave McCracken
Oracle Corp.
Quoted 6 lines · Dave McCracken
One outstanding issue I see is how to handle readonly mappings. If we follow the model of regular page typecount, readonly mappings of superpages would not conflict with the "conflicts with superpage" type. This means a subsequent attempt to change it to a read/write mapping could fail, just like with a regular page. Or we could count all mappings of superpages as if they were read/write.
I'd keep an extra refcount in superpage_info to track read-only mappings (or all superpage mappings, as page->count_info does for 4kB mappings). It's trivial extra space and avoids having unexpected extra restrictions on read-only superpage mappings.
Quoted 2 lines
What are your thoughts? It seems fairly simple and elegant to me, and at this point I don't see any big holes in it.
It does mean that creating/destroying pagetable pages causes an extra locked read-modify-write cycle on a non-local cacheline (superpage_info refcount). Would this be significant? Not sure. I guess we'd only be doing it for guests with the superpage capability configured, and we could do some performance comparisons with the capability enabled/disabled. I think overall I quite like your suggestion.
-- Keir
Quoted 10 lines · Keir Fraser
Quoted 3 lines
What are your thoughts? It seems fairly simple and elegant to me, and at this
point I don't see any big holes in it.It does mean that creating/destroying pagetable pages causes an extra locked read-modify-write cycle on a non-local cacheline (
superpage_inforefcount). Would this be significant? Not sure. I guess we'd only be doing it for guests with the superpage capability configured, and we could do some performance comparisons with the capability enabled/disabled. I think overall I quite like your suggestion.
Oh, now I think about it, although your suggestion deals with type conflicts, it doesn't handle page lifetimes. What if a page is only mapped as a superpage? The page->count_info would not be incremented by the superpage mappings, and the page would be erroneously freed to the Xen free pools? So I'm not so sure we can so easily avoid the mess-with-every-page's-refcount on first mapping of a superpage... :-(
-- Keir
Quoted 6 lines · Keir Fraser
Oh, now I think about it, although your suggestion deals with type conflicts, it doesn't handle page lifetimes. What if a page is only mapped as a superpage? The
page->count_infowould not be incremented by the superpage mappings, and the page would be erroneously freed to the Xen free pools? So I'm not so sure we can so easily avoid the mess-with-every-page's-refcount on first mapping of a superpage... :-(
It should be simple enough to also check superpage->count_info in those places. So the total mappings of a page would be page->count_info + superpage->count_info. Good thing you suggested we also have a count in the superpage_info struct :)
Dave
Quoted 8 lines · Dave McCracken
Quoted 6 lines · Keir Fraser
Oh, now I think about it, although your suggestion deals with type conflicts, it doesn't handle page lifetimes. What if a page is only mapped as a superpage? The
page->count_infowould not be incremented by the superpage mappings, and the page would be erroneously freed to the Xen free pools? So I'm not so sure we can so easily avoid the mess-with-every-page's-refcount on first mapping of a superpage... :-(
Quoted 4 lines
It should be simple enough to also check
superpage->count_infoin those places. So the total mappings of a page would bepage->count_info+superpage->count_info. Good thing you suggested we also have a count in thesuperpage_infostruct :)
I think you're going to have trouble handling two separate reference counts, for superpages and single pages, in a race-free manner that is any better than checking/updating reference counts across all pages in a superpage on first superpage mapping.
-- Keir
Quoted 9 lines · Keir Fraser
Quoted 4 lines
It should be simple enough to also check
superpage->count_infoin those places. So the total mappings of a page would bepage->count_info+superpage->count_info. Good thing you suggested we also have a count in thesuperpage_infostruct :)I think you're going to have trouble handling two separate reference counts, for superpages and single pages, in a race-free manner that is any better than checking/updating reference counts across all pages in a superpage on first superpage mapping.
For example: When making first superpage mapping, how do you know that all pages belong to the relevant domain, without scanning every page_info? When destructing last superpage mapping (or single-page mapping) how do you safely check the 'other' reference count to decide whether the page is freeable, without having races (last single-page and superpage mappings could be destructed concurrently, need to ensure any given page gets freed exactly once). And I could think of others no doubt... Just pointing out how careful you have to be if you think you can avoid the naïve refcount-updatign algorithms I suggested. I'd rather shoot down the obvious races before you do the coding.
-- Keir