Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements
This series adjusts the error reporting in the various code paths, with the intention that fatal errors can be detected by callers and handled properly. During my performance analysis with callgrind I found and fixed a few bottlenecks in the page-in code paths. Patches 1, 2 and 3 were already sent earlier. I''m including them here again since later patches depend on them. Changes: xenpaging: use flat index for pagefile and page-in requests xenpaging: no poll timeout while page-out is in progress xenpaging: mmap guest pages read-only xenpaging: reduce number of qemu cache flushes xenpaging: move nominate+evict into single function xenpaging: improve performance in policy_choose_victim xenpaging: unify error handling xenpaging: move pagefile filedescriptor into struct xenpaging xenpaging: move page_buffer into struct xenpaging xenpaging: implement stack of free slots in pagefile tools/xenpaging/policy.h | 2 tools/xenpaging/policy_default.c | 63 ++++-- tools/xenpaging/xenpaging.c | 353 ++++++++++++++++++++++----------------- tools/xenpaging/xenpaging.h | 16 - 4 files changed, 261 insertions(+), 173 deletions(-)
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 01 of 10] xenpaging: use flat index for pagefile and page-in requests
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937786 -3600 # Node ID 2d71d8bca0f15d353d28f5a42cc0c4e21002e8d5 # Parent 39df93acd4089eaae138e1decf49aec39fb19417 xenpaging: use flat index for pagefile and page-in requests This change is based on an idea by <hongkaixing@huawei.com> and <bicky.shi@huawei.com>. Scanning the victims[] array is time consuming with a large number of target pages. Replace the loop to find the slot in the pagefile which holds the requested gfn with an index. Remove the victims array and replace it with a flat array. This array holds the gfn for a given slot in the pagefile. Adjust all users of the victims array. Rename variable in main() from i to slot to clearify the meaning. Update xenpaging_evict_page() to pass a pointer to xen_pfn_t to xc_map_foreign_pages(). Update policy_choose_victim() to return either a gfn or INVALID_MFN. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 39df93acd408 -r 2d71d8bca0f1 tools/xenpaging/policy.h --- a/tools/xenpaging/policy.h +++ b/tools/xenpaging/policy.h @@ -29,7 +29,7 @@ int policy_init(struct xenpaging *paging); -int policy_choose_victim(struct xenpaging *paging, struct victim *victim); +unsigned long policy_choose_victim(struct xenpaging *paging); void policy_notify_paged_out(unsigned long gfn); void policy_notify_paged_in(unsigned long gfn); void policy_notify_paged_in_nomru(unsigned long gfn); diff -r 39df93acd408 -r 2d71d8bca0f1 tools/xenpaging/policy_default.c --- a/tools/xenpaging/policy_default.c +++ b/tools/xenpaging/policy_default.c @@ -77,7 +77,7 @@ int policy_init(struct xenpaging *paging return rc; } -int policy_choose_victim(struct xenpaging *paging, struct victim *victim) +unsigned long policy_choose_victim(struct xenpaging *paging) { xc_interface *xch = paging->xc_handle; unsigned long wrap = current_gfn; @@ -102,16 +102,13 @@ int policy_choose_victim(struct xenpagin /* One more round before returning ENOSPC */ continue; } - victim->gfn = INVALID_MFN; - return -ENOSPC; + return INVALID_MFN; } } while ( test_bit(current_gfn, bitmap) || test_bit(current_gfn, unconsumed) ); set_bit(current_gfn, unconsumed); - victim->gfn = current_gfn; - - return 0; + return current_gfn; } void policy_notify_paged_out(unsigned long gfn) diff -r 39df93acd408 -r 2d71d8bca0f1 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -430,6 +430,12 @@ static struct xenpaging *xenpaging_init( } DPRINTF("max_pages = %d\n", paging->max_pages); + /* Allocate indicies for pagefile slots */ + paging->slot_to_gfn = calloc(paging->max_pages, sizeof(*paging->slot_to_gfn)); + paging->gfn_to_slot = calloc(paging->max_pages, sizeof(*paging->gfn_to_slot)); + if ( !paging->slot_to_gfn || !paging->gfn_to_slot ) + goto err; + /* Initialise policy */ rc = policy_init(paging); if ( rc != 0 ) @@ -468,6 +474,8 @@ static struct xenpaging *xenpaging_init( free(dom_path); free(watch_target_tot_pages); + free(paging->slot_to_gfn); + free(paging->gfn_to_slot); free(paging->bitmap); free(paging); } @@ -561,31 +569,29 @@ static void put_response(struct mem_even RING_PUSH_RESPONSES(back_ring); } -static int xenpaging_evict_page(struct xenpaging *paging, struct victim *victim, int fd, int i) +static int xenpaging_evict_page(struct xenpaging *paging, unsigned long gfn, int fd, int slot) { xc_interface *xch = paging->xc_handle; void *page; - unsigned long gfn; + xen_pfn_t victim = gfn; int ret; DECLARE_DOMCTL; /* Map page */ - gfn = victim->gfn; ret = -EFAULT; - page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, - PROT_READ | PROT_WRITE, &gfn, 1); + page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, PROT_READ | PROT_WRITE, &victim, 1); if ( page == NULL ) { - PERROR("Error mapping page %lx", victim->gfn); + PERROR("Error mapping page %lx", gfn); goto out; } /* Copy page */ - ret = write_page(fd, page, i); + ret = write_page(fd, page, slot); if ( ret != 0 ) { - PERROR("Error copying page %lx", victim->gfn); + PERROR("Error copying page %lx", gfn); munmap(page, PAGE_SIZE); goto out; } @@ -593,17 +599,20 @@ static int xenpaging_evict_page(struct x munmap(page, PAGE_SIZE); /* Tell Xen to evict page */ - ret = xc_mem_paging_evict(xch, paging->mem_event.domain_id, - victim->gfn); + ret = xc_mem_paging_evict(xch, paging->mem_event.domain_id, gfn); if ( ret != 0 ) { - PERROR("Error evicting page %lx", victim->gfn); + PERROR("Error evicting page %lx", gfn); goto out; } - DPRINTF("evict_page > gfn %lx pageslot %d\n", victim->gfn, i); + DPRINTF("evict_page > gfn %lx pageslot %d\n", gfn, slot); /* Notify policy of page being paged out */ - policy_notify_paged_out(victim->gfn); + policy_notify_paged_out(gfn); + + /* Update index */ + paging->slot_to_gfn[slot] = gfn; + paging->gfn_to_slot[gfn] = slot; /* Record number of evicted pages */ paging->num_paged_out++; @@ -710,19 +719,19 @@ static void resume_pages(struct xenpagin page_in_trigger(); } -static int evict_victim(struct xenpaging *paging, struct victim *victim, int fd, int i) +static int evict_victim(struct xenpaging *paging, int fd, int slot) { xc_interface *xch = paging->xc_handle; + unsigned long gfn; int j = 0; int ret; do { - ret = policy_choose_victim(paging, victim); - if ( ret != 0 ) + gfn = policy_choose_victim(paging); + if ( gfn == INVALID_MFN ) { - if ( ret != -ENOSPC ) - ERROR("Error choosing victim"); + ret = -ENOSPC; goto out; } @@ -731,9 +740,9 @@ static int evict_victim(struct xenpaging ret = -EINTR; goto out; } - ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, victim->gfn); + ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, gfn); if ( ret == 0 ) - ret = xenpaging_evict_page(paging, victim, fd, i); + ret = xenpaging_evict_page(paging, gfn, fd, slot); else { if ( j++ % 1000 == 0 ) @@ -743,7 +752,7 @@ static int evict_victim(struct xenpaging } while ( ret ); - if ( test_and_set_bit(victim->gfn, paging->bitmap) ) + if ( test_and_set_bit(gfn, paging->bitmap) ) ERROR("Page has been evicted before"); ret = 0; @@ -753,7 +762,7 @@ static int evict_victim(struct xenpaging } /* Evict a batch of pages and write them to a free slot in the paging file */ -static int evict_pages(struct xenpaging *paging, int fd, struct victim *victims, int num_pages) +static int evict_pages(struct xenpaging *paging, int fd, int num_pages) { xc_interface *xch = paging->xc_handle; int rc, slot, num = 0; @@ -761,10 +770,10 @@ static int evict_pages(struct xenpaging for ( slot = 0; slot < paging->max_pages && num < num_pages; slot++ ) { /* Slot is allocated */ - if ( victims[slot].gfn != INVALID_MFN ) + if ( paging->slot_to_gfn[slot] ) continue; - rc = evict_victim(paging, &victims[slot], fd, slot); + rc = evict_victim(paging, fd, slot); if ( rc == -ENOSPC ) break; if ( rc == -EINTR ) @@ -780,11 +789,10 @@ int main(int argc, char *argv[]) { struct sigaction act; struct xenpaging *paging; - struct victim *victims; mem_event_request_t req; mem_event_response_t rsp; int num, prev_num = 0; - int i; + int slot; int tot_pages; int rc = -1; int rc1; @@ -813,15 +821,6 @@ int main(int argc, char *argv[]) return 2; } - /* Allocate upper limit of pages to allow growing and shrinking */ - victims = calloc(paging->max_pages, sizeof(struct victim)); - if ( !victims ) - goto out; - - /* Mark all slots as unallocated */ - for ( i = 0; i < paging->max_pages; i++ ) - victims[i].gfn = INVALID_MFN; - /* ensure that if we get a signal, we''ll do cleanup, then exit */ act.sa_handler = close_handler; act.sa_flags = 0; @@ -853,32 +852,35 @@ int main(int argc, char *argv[]) { get_request(&paging->mem_event, &req); + if ( req.gfn > paging->max_pages ) + { + ERROR("Requested gfn %"PRIx64" higher than max_pages %lx\n", req.gfn, paging->max_pages); + goto out; + } + /* Check if the page has already been paged in */ if ( test_and_clear_bit(req.gfn, paging->bitmap) ) { /* Find where in the paging file to read from */ - for ( i = 0; i < paging->max_pages; i++ ) + slot = paging->gfn_to_slot[req.gfn]; + + /* Sanity check */ + if ( paging->slot_to_gfn[slot] != req.gfn ) { - if ( victims[i].gfn == req.gfn ) - break; - } - - if ( i >= paging->max_pages ) - { - DPRINTF("Couldn''t find page %"PRIx64"\n", req.gfn); + ERROR("Expected gfn %"PRIx64" in slot %d, but found gfn %lx\n", req.gfn, slot, paging->slot_to_gfn[slot]); goto out; } - + if ( req.flags & MEM_EVENT_FLAG_DROP_PAGE ) { - DPRINTF("drop_page ^ gfn %"PRIx64" pageslot %d\n", req.gfn, i); + DPRINTF("drop_page ^ gfn %"PRIx64" pageslot %d\n", req.gfn, slot); /* Notify policy of page being dropped */ policy_notify_dropped(req.gfn); } else { /* Populate the page */ - rc = xenpaging_populate_page(paging, req.gfn, fd, i); + rc = xenpaging_populate_page(paging, req.gfn, fd, slot); if ( rc != 0 ) { PERROR("Error populating page %"PRIx64"", req.gfn); @@ -899,7 +901,7 @@ int main(int argc, char *argv[]) } /* Clear this pagefile slot */ - victims[i].gfn = INVALID_MFN; + paging->slot_to_gfn[slot] = 0; } else { @@ -969,7 +971,7 @@ int main(int argc, char *argv[]) /* Limit the number of evicts to be able to process page-in requests */ if ( num > 42 ) num = 42; - evict_pages(paging, fd, victims, num); + evict_pages(paging, fd, num); } /* Resume some pages if target not reached */ else if ( tot_pages < paging->target_tot_pages && paging->num_paged_out ) @@ -989,7 +991,6 @@ int main(int argc, char *argv[]) out: close(fd); unlink_pagefile(); - free(victims); /* Tear down domain paging */ rc1 = xenpaging_teardown(paging); diff -r 39df93acd408 -r 2d71d8bca0f1 tools/xenpaging/xenpaging.h --- a/tools/xenpaging/xenpaging.h +++ b/tools/xenpaging/xenpaging.h @@ -46,6 +46,9 @@ struct xenpaging { unsigned long *bitmap; + unsigned long *slot_to_gfn; + int *gfn_to_slot; + struct mem_event mem_event; /* number of pages for which data structures were allocated */ int max_pages; @@ -56,13 +59,6 @@ struct xenpaging { unsigned long pagein_queue[XENPAGING_PAGEIN_QUEUE_SIZE]; }; - -struct victim { - /* the gfn of the page to evict */ - unsigned long gfn; -}; - - extern void create_page_in_thread(struct xenpaging *paging); extern void page_in_trigger(void);
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 02 of 10] xenpaging: no poll timeout while page-out is in progress
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937792 -3600 # Node ID 55b2e9676a5a871833e6c57ec3b424c70f15af2b # Parent 2d71d8bca0f15d353d28f5a42cc0c4e21002e8d5 xenpaging: no poll timeout while page-out is in progress The main loop calls xenpaging_wait_for_event_or_timeout() unconditionally before doing any work. This function calls poll() with a timeout of 100ms. As a result the page-out process is very slow due to the delay in poll(). Call poll() without timeout so that it returns immediately until the page-out is done. Page-out is done when either the policy finds no more pages to nominate or when the requested number of pages is reached. The condition is cleared when a watch event arrives, so that processing the new target is not delayed once again by poll(). v2: - no poll timeout also when large number of evicts is pending Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 2d71d8bca0f1 -r 55b2e9676a5a tools/xenpaging/policy_default.c --- a/tools/xenpaging/policy_default.c +++ b/tools/xenpaging/policy_default.c @@ -90,6 +90,7 @@ unsigned long policy_choose_victim(struc /* Could not nominate any gfn */ if ( wrap == current_gfn ) { + paging->use_poll_timeout = 1; /* Count wrap arounds */ unconsumed_cleared++; /* Force retry every few seconds (depends on poll() timeout) */ diff -r 2d71d8bca0f1 -r 55b2e9676a5a tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -84,6 +84,7 @@ static int xenpaging_wait_for_event_or_t struct pollfd fd[2]; int port; int rc; + int timeout; /* Wait for event channel and xenstore */ fd[0].fd = xc_evtchn_fd(xce); @@ -91,7 +92,9 @@ static int xenpaging_wait_for_event_or_t fd[1].fd = xs_fileno(paging->xs_handle); fd[1].events = POLLIN | POLLERR; - rc = poll(fd, 2, 100); + /* No timeout while page-out is still in progress */ + timeout = paging->use_poll_timeout ? 100 : 0; + rc = poll(fd, 2, timeout); if ( rc < 0 ) { if (errno == EINTR) @@ -133,6 +136,8 @@ static int xenpaging_wait_for_event_or_t if ( target_tot_pages < 0 || target_tot_pages > paging->max_pages ) target_tot_pages = paging->max_pages; paging->target_tot_pages = target_tot_pages; + /* Disable poll() delay while new target is not yet reached */ + paging->use_poll_timeout = 0; DPRINTF("new target_tot_pages %d\n", target_tot_pages); } free(val); @@ -970,7 +975,10 @@ int main(int argc, char *argv[]) } /* Limit the number of evicts to be able to process page-in requests */ if ( num > 42 ) + { + paging->use_poll_timeout = 0; num = 42; + } evict_pages(paging, fd, num); } /* Resume some pages if target not reached */ @@ -984,6 +992,11 @@ int main(int argc, char *argv[]) } resume_pages(paging, num); } + /* Now target was reached, enable poll() timeout */ + else + { + paging->use_poll_timeout = 1; + } } DPRINTF("xenpaging got signal %d\n", interrupted); diff -r 2d71d8bca0f1 -r 55b2e9676a5a tools/xenpaging/xenpaging.h --- a/tools/xenpaging/xenpaging.h +++ b/tools/xenpaging/xenpaging.h @@ -55,6 +55,7 @@ struct xenpaging { int num_paged_out; int target_tot_pages; int policy_mru_size; + int use_poll_timeout; int debug; unsigned long pagein_queue[XENPAGING_PAGEIN_QUEUE_SIZE]; };
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937794 -3600 # Node ID dcf9cca8e1d5121782ea96dc183f52ff0cee2251 # Parent 55b2e9676a5a871833e6c57ec3b424c70f15af2b xenpaging: mmap guest pages read-only xenpaging does not write to the gfn, so map the gfn to page-out in read-only mode. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 55b2e9676a5a -r dcf9cca8e1d5 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -585,7 +585,7 @@ static int xenpaging_evict_page(struct x /* Map page */ ret = -EFAULT; - page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, PROT_READ | PROT_WRITE, &victim, 1); + page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, PROT_READ, &victim, 1); if ( page == NULL ) { PERROR("Error mapping page %lx", gfn);
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 04 of 10] xenpaging: reduce number of qemu cache flushes
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937795 -3600 # Node ID e2092e841d281fcef8abb005cc41e23af09f1db3 # Parent dcf9cca8e1d5121782ea96dc183f52ff0cee2251 xenpaging: reduce number of qemu cache flushes Currently the command to flush the qemu cache is called alot if there are no more pages to evict. This causes churn in the logfiles, and qemu can not release more pages anyway since the last command. Fix this by remembering the current number of paged-out gfns, if this number did not change since the last flush command then sending another new flush command will not free any more gfns. Remove return code from xenpaging_mem_paging_flush_ioemu_cache() since errors do not matter, and will be handled elsewhere. Also failure to send the flush command is not fatal. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r dcf9cca8e1d5 -r e2092e841d28 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -61,18 +61,15 @@ static void close_handler(int sig) unlink_pagefile(); } -static int xenpaging_mem_paging_flush_ioemu_cache(struct xenpaging *paging) +static void xenpaging_mem_paging_flush_ioemu_cache(struct xenpaging *paging) { struct xs_handle *xsh = paging->xs_handle; domid_t domain_id = paging->mem_event.domain_id; char path[80]; - bool rc; sprintf(path, "/local/domain/0/device-model/%u/command", domain_id); - rc = xs_write(xsh, XBT_NULL, path, "flush-cache", strlen("flush-cache")); - - return rc == true ? 0 : -1; + xs_write(xsh, XBT_NULL, path, "flush-cache", strlen("flush-cache")); } static int xenpaging_wait_for_event_or_timeout(struct xenpaging *paging) @@ -728,7 +725,7 @@ static int evict_victim(struct xenpaging { xc_interface *xch = paging->xc_handle; unsigned long gfn; - int j = 0; + static int num_paged_out; int ret; do @@ -736,6 +733,17 @@ static int evict_victim(struct xenpaging gfn = policy_choose_victim(paging); if ( gfn == INVALID_MFN ) { + /* If the number did not change after last flush command then + * the command did not reach qemu yet, or qemu still processes + * the command, or qemu has nothing to release. + * Right now there is no need to issue the command again. + */ + if ( num_paged_out != paging->num_paged_out ) + { + DPRINTF("Flushing qemu cache\n"); + xenpaging_mem_paging_flush_ioemu_cache(paging); + num_paged_out = paging->num_paged_out; + } ret = -ENOSPC; goto out; } @@ -748,12 +756,6 @@ static int evict_victim(struct xenpaging ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, gfn); if ( ret == 0 ) ret = xenpaging_evict_page(paging, gfn, fd, slot); - else - { - if ( j++ % 1000 == 0 ) - if ( xenpaging_mem_paging_flush_ioemu_cache(paging) ) - PERROR("Error flushing ioemu cache"); - } } while ( ret );
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 05 of 10] xenpaging: move nominate+evict into single function
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937797 -3600 # Node ID 2e9a4bfaf79d64863f18a519c448afc54315aa1d # Parent e2092e841d281fcef8abb005cc41e23af09f1db3 xenpaging: move nominate+evict into single function Move all code to evict a single gfn into one function. This simplifies error handling in caller. The function returns -1 on fatal error, 0 on success and 1 if the gfn cant be paged. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r e2092e841d28 -r 2e9a4bfaf79d tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -571,6 +571,11 @@ static void put_response(struct mem_even RING_PUSH_RESPONSES(back_ring); } +/* Evict a given gfn + * Returns < 0 on fatal error + * Returns 0 on successful evict + * Returns > 0 if gfn can not be evicted + */ static int xenpaging_evict_page(struct xenpaging *paging, unsigned long gfn, int fd, int slot) { xc_interface *xch = paging->xc_handle; @@ -580,31 +585,51 @@ static int xenpaging_evict_page(struct x DECLARE_DOMCTL; + /* Nominate page */ + ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, gfn); + if ( ret < 0 ) + { + /* unpageable gfn is indicated by EBUSY */ + if ( errno == EBUSY ) + ret = 1; + else + PERROR("Error nominating page %lx", gfn); + goto out; + } + /* Map page */ - ret = -EFAULT; page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, PROT_READ, &victim, 1); if ( page == NULL ) { PERROR("Error mapping page %lx", gfn); + ret = -1; goto out; } /* Copy page */ ret = write_page(fd, page, slot); - if ( ret != 0 ) + if ( ret < 0 ) { PERROR("Error copying page %lx", gfn); munmap(page, PAGE_SIZE); + ret = -1; goto out; } + /* Release page */ munmap(page, PAGE_SIZE); /* Tell Xen to evict page */ ret = xc_mem_paging_evict(xch, paging->mem_event.domain_id, gfn); - if ( ret != 0 ) + if ( ret < 0 ) { - PERROR("Error evicting page %lx", gfn); + /* A gfn in use is indicated by EBUSY */ + if ( errno == EBUSY ) + { + ret = 1; + DPRINTF("Nominated page %lx busy", gfn); + } else + PERROR("Error evicting page %lx", gfn); goto out; } @@ -619,6 +644,8 @@ static int xenpaging_evict_page(struct x /* Record number of evicted pages */ paging->num_paged_out++; + ret = 0; + out: return ret; } @@ -753,9 +780,10 @@ static int evict_victim(struct xenpaging ret = -EINTR; goto out; } - ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, gfn); - if ( ret == 0 ) - ret = xenpaging_evict_page(paging, gfn, fd, slot); + + ret = xenpaging_evict_page(paging, gfn, fd, slot); + if ( ret < 0 ) + goto out; } while ( ret );
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 06 of 10] xenpaging: improve performance in policy_choose_victim
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937798 -3600 # Node ID b7906ad28153042726866cfbef40f8adf8dc1520 # Parent 2e9a4bfaf79d64863f18a519c448afc54315aa1d xenpaging: improve performance in policy_choose_victim policy_choose_victim() is one of the bottlenecks in xenpaging. It is called alot to find free bits in the fragmented bitmaps. Reduce turnaround time by skipping longs with all bits set. Adjust wrap detection in loop. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 2e9a4bfaf79d -r b7906ad28153 tools/xenpaging/policy_default.c --- a/tools/xenpaging/policy_default.c +++ b/tools/xenpaging/policy_default.c @@ -80,33 +80,58 @@ int policy_init(struct xenpaging *paging unsigned long policy_choose_victim(struct xenpaging *paging) { xc_interface *xch = paging->xc_handle; - unsigned long wrap = current_gfn; + unsigned long i; - do + /* One iteration over all possible gfns */ + for ( i = 0; i < max_pages; i++ ) { + /* Try next gfn */ current_gfn++; + + /* Restart on wrap */ if ( current_gfn >= max_pages ) current_gfn = 0; - /* Could not nominate any gfn */ - if ( wrap == current_gfn ) + + if ( (current_gfn & (BITS_PER_LONG - 1)) == 0 ) { - paging->use_poll_timeout = 1; - /* Count wrap arounds */ - unconsumed_cleared++; - /* Force retry every few seconds (depends on poll() timeout) */ - if ( unconsumed_cleared > 123) + /* All gfns busy */ + if ( ~bitmap[current_gfn >> ORDER_LONG] == 0 || ~unconsumed[current_gfn >> ORDER_LONG] == 0 ) { - /* Force retry of unconsumed gfns */ - bitmap_clear(unconsumed, max_pages); - unconsumed_cleared = 0; - DPRINTF("clearing unconsumed, wrap %lx", wrap); - /* One more round before returning ENOSPC */ + current_gfn += BITS_PER_LONG; + i += BITS_PER_LONG; continue; } - return INVALID_MFN; } + + /* gfn busy */ + if ( test_bit(current_gfn, bitmap) ) + continue; + + /* gfn already tested */ + if ( test_bit(current_gfn, bitmap) ) + continue; + + /* gfn found */ + break; } - while ( test_bit(current_gfn, bitmap) || test_bit(current_gfn, unconsumed) ); + + /* Could not nominate any gfn */ + if ( i >= max_pages ) + { + /* No more pages, wait in poll */ + paging->use_poll_timeout = 1; + /* Count wrap arounds */ + unconsumed_cleared++; + /* Force retry every few seconds (depends on poll() timeout) */ + if ( unconsumed_cleared > 123) + { + /* Force retry of unconsumed gfns on next call */ + bitmap_clear(unconsumed, max_pages); + unconsumed_cleared = 0; + DPRINTF("clearing unconsumed, current_gfn %lx", current_gfn); + } + return INVALID_MFN; + } set_bit(current_gfn, unconsumed); return current_gfn;
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937800 -3600 # Node ID 649791966e1a8fb5e363d8745250bcd120871800 # Parent b7906ad28153042726866cfbef40f8adf8dc1520 xenpaging: unify error handling Update functions to return -1 on error, 0 on success. Simplify init_page() and make sure errno is assigned. Adjust PERROR/ERROR usage, use PERROR early because it overwrites errno. Adjust xenpaging_populate_page() to handle gfn as unsigned long. Update xenpaging exit code handling. xenpaging_teardown cant possible fail. Adjust mainloop to indicate possible errors to final exit. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r b7906ad28153 -r 649791966e1a tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -98,7 +98,7 @@ static int xenpaging_wait_for_event_or_t return 0; PERROR("Poll exited with an error"); - return -errno; + return -1; } /* First check for guest shutdown */ @@ -116,6 +116,7 @@ static int xenpaging_wait_for_event_or_t { xs_unwatch(paging->xs_handle, "@releaseDomain", watch_token); interrupted = SIGQUIT; + /* No further poll result processing */ rc = 0; } } @@ -183,26 +184,20 @@ static int xenpaging_get_tot_pages(struc static void *init_page(void) { void *buffer; - int ret; /* Allocated page memory */ - ret = posix_memalign(&buffer, PAGE_SIZE, PAGE_SIZE); - if ( ret != 0 ) - goto out_alloc; + errno = posix_memalign(&buffer, PAGE_SIZE, PAGE_SIZE); + if ( errno != 0 ) + return NULL; /* Lock buffer in memory so it can''t be paged out */ - ret = mlock(buffer, PAGE_SIZE); - if ( ret != 0 ) - goto out_lock; + if ( mlock(buffer, PAGE_SIZE) < 0 ) + { + free(buffer); + buffer = NULL; + } return buffer; - - out_init: - munlock(buffer, PAGE_SIZE); - out_lock: - free(buffer); - out_alloc: - return NULL; } static void usage(void) @@ -449,7 +444,7 @@ static struct xenpaging *xenpaging_init( paging_buffer = init_page(); if ( !paging_buffer ) { - ERROR("Creating page aligned load buffer"); + PERROR("Creating page aligned load buffer"); goto err; } @@ -485,18 +480,14 @@ static struct xenpaging *xenpaging_init( return NULL; } -static int xenpaging_teardown(struct xenpaging *paging) +static void xenpaging_teardown(struct xenpaging *paging) { int rc; - xc_interface *xch; - - if ( paging == NULL ) - return 0; + xc_interface *xch = paging->xc_handle; xs_unwatch(paging->xs_handle, watch_target_tot_pages, ""); xs_unwatch(paging->xs_handle, "@releaseDomain", watch_token); - xch = paging->xc_handle; paging->xc_handle = NULL; /* Tear down domain paging in Xen */ rc = xc_mem_paging_disable(xch, paging->mem_event.domain_id); @@ -528,13 +519,8 @@ static int xenpaging_teardown(struct xen rc = xc_interface_close(xch); if ( rc != 0 ) { - PERROR("Error closing connection to xen"); + ERROR("Error closing connection to xen"); } - - return 0; - - err: - return -1; } static void get_request(struct mem_event *mem_event, mem_event_request_t *req) @@ -684,21 +670,19 @@ static int xenpaging_resume_page(struct return ret; } -static int xenpaging_populate_page(struct xenpaging *paging, - xen_pfn_t gfn, int fd, int i) +static int xenpaging_populate_page(struct xenpaging *paging, unsigned long gfn, int fd, int i) { xc_interface *xch = paging->xc_handle; - void *page; int ret; unsigned char oom = 0; - DPRINTF("populate_page < gfn %"PRI_xen_pfn" pageslot %d\n", gfn, i); + DPRINTF("populate_page < gfn %lx pageslot %d\n", gfn, i); /* Read page */ ret = read_page(fd, paging_buffer, i); if ( ret != 0 ) { - ERROR("Error reading page"); + PERROR("Error reading page"); goto out; } @@ -707,17 +691,18 @@ static int xenpaging_populate_page(struc /* Tell Xen to allocate a page for the domain */ ret = xc_mem_paging_load(xch, paging->mem_event.domain_id, gfn, paging_buffer); - if ( ret != 0 ) + if ( ret < 0 ) { if ( errno == ENOMEM ) { if ( oom++ == 0 ) - DPRINTF("ENOMEM while preparing gfn %"PRI_xen_pfn"\n", gfn); + DPRINTF("ENOMEM while preparing gfn %lx\n", gfn); sleep(1); continue; } - PERROR("Error loading %"PRI_xen_pfn" during page-in", gfn); - goto out; + PERROR("Error loading %lx during page-in", gfn); + ret = -1; + break; } } while ( ret && !interrupted ); @@ -748,6 +733,11 @@ static void resume_pages(struct xenpagin page_in_trigger(); } +/* Evict one gfn and write it to the given slot + * Returns < 0 on fatal error + * Returns 0 on successful evict + * Returns > 0 if no gfn can be evicted + */ static int evict_victim(struct xenpaging *paging, int fd, int slot) { xc_interface *xch = paging->xc_handle; @@ -771,13 +761,13 @@ static int evict_victim(struct xenpaging xenpaging_mem_paging_flush_ioemu_cache(paging); num_paged_out = paging->num_paged_out; } - ret = -ENOSPC; + ret = ENOSPC; goto out; } if ( interrupted ) { - ret = -EINTR; + ret = EINTR; goto out; } @@ -788,7 +778,7 @@ static int evict_victim(struct xenpaging while ( ret ); if ( test_and_set_bit(gfn, paging->bitmap) ) - ERROR("Page has been evicted before"); + ERROR("Page %lx has been evicted before", gfn); ret = 0; @@ -796,7 +786,11 @@ static int evict_victim(struct xenpaging return ret; } -/* Evict a batch of pages and write them to a free slot in the paging file */ +/* Evict a batch of pages and write them to a free slot in the paging file + * Returns < 0 on fatal error + * Returns 0 if no gfn can be evicted + * Returns > 0 on successful evict + */ static int evict_pages(struct xenpaging *paging, int fd, int num_pages) { xc_interface *xch = paging->xc_handle; @@ -809,12 +803,12 @@ static int evict_pages(struct xenpaging continue; rc = evict_victim(paging, fd, slot); - if ( rc == -ENOSPC ) + if ( rc ) + { + num = rc < 0 ? -1 : num; break; - if ( rc == -EINTR ) - break; - if ( num && num % 100 == 0 ) - DPRINTF("%d pages evicted\n", num); + } + num++; } return num; @@ -829,8 +823,7 @@ int main(int argc, char *argv[]) int num, prev_num = 0; int slot; int tot_pages; - int rc = -1; - int rc1; + int rc; xc_interface *xch; int open_flags = O_CREAT | O_TRUNC | O_RDWR; @@ -875,7 +868,7 @@ int main(int argc, char *argv[]) rc = xenpaging_wait_for_event_or_timeout(paging); if ( rc < 0 ) { - PERROR("Error getting event"); + ERROR("Error getting event"); goto out; } else if ( rc != 0 ) @@ -885,6 +878,9 @@ int main(int argc, char *argv[]) while ( RING_HAS_UNCONSUMED_REQUESTS(&paging->mem_event.back_ring) ) { + /* Indicate possible error */ + rc = 1; + get_request(&paging->mem_event, &req); if ( req.gfn > paging->max_pages ) @@ -915,10 +911,9 @@ int main(int argc, char *argv[]) else { /* Populate the page */ - rc = xenpaging_populate_page(paging, req.gfn, fd, slot); - if ( rc != 0 ) + if ( xenpaging_populate_page(paging, req.gfn, fd, slot) < 0 ) { - PERROR("Error populating page %"PRIx64"", req.gfn); + ERROR("Error populating page %"PRIx64"", req.gfn); goto out; } } @@ -928,8 +923,7 @@ int main(int argc, char *argv[]) rsp.vcpu_id = req.vcpu_id; rsp.flags = req.flags; - rc = xenpaging_resume_page(paging, &rsp, 1); - if ( rc != 0 ) + if ( xenpaging_resume_page(paging, &rsp, 1) < 0 ) { PERROR("Error resuming page %"PRIx64"", req.gfn); goto out; @@ -955,8 +949,7 @@ int main(int argc, char *argv[]) rsp.vcpu_id = req.vcpu_id; rsp.flags = req.flags; - rc = xenpaging_resume_page(paging, &rsp, 0); - if ( rc != 0 ) + if ( xenpaging_resume_page(paging, &rsp, 0) < 0 ) { PERROR("Error resuming page %"PRIx64"", req.gfn); goto out; @@ -983,6 +976,9 @@ int main(int argc, char *argv[]) if ( interrupted ) break; + /* Indicate possible error */ + rc = 1; + /* Check if the target has been reached already */ tot_pages = xenpaging_get_tot_pages(paging); if ( tot_pages < 0 ) @@ -1009,7 +1005,8 @@ int main(int argc, char *argv[]) paging->use_poll_timeout = 0; num = 42; } - evict_pages(paging, fd, num); + if ( evict_pages(paging, fd, num) < 0 ) + goto out; } /* Resume some pages if target not reached */ else if ( tot_pages < paging->target_tot_pages && paging->num_paged_out ) @@ -1029,6 +1026,10 @@ int main(int argc, char *argv[]) } } + + /* No error */ + rc = 0; + DPRINTF("xenpaging got signal %d\n", interrupted); out: @@ -1036,13 +1037,9 @@ int main(int argc, char *argv[]) unlink_pagefile(); /* Tear down domain paging */ - rc1 = xenpaging_teardown(paging); - if ( rc1 != 0 ) - fprintf(stderr, "Error tearing down paging"); + xenpaging_teardown(paging); - if ( rc == 0 ) - rc = rc1; - return rc; + return rc ? 2 : 0; }
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 08 of 10] xenpaging: move pagefile filedescriptor into struct xenpaging
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937802 -3600 # Node ID 4c494f4b99f4da449fe72cd6ba6398e666231d9b # Parent 649791966e1a8fb5e363d8745250bcd120871800 xenpaging: move pagefile filedescriptor into struct xenpaging Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 649791966e1a -r 4c494f4b99f4 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -448,6 +448,14 @@ static struct xenpaging *xenpaging_init( goto err; } + /* Open file */ + paging->fd = open(filename, O_CREAT | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR); + if ( paging->fd < 0 ) + { + PERROR("failed to open file"); + goto err; + } + return paging; err: @@ -562,7 +570,7 @@ static void put_response(struct mem_even * Returns 0 on successful evict * Returns > 0 if gfn can not be evicted */ -static int xenpaging_evict_page(struct xenpaging *paging, unsigned long gfn, int fd, int slot) +static int xenpaging_evict_page(struct xenpaging *paging, unsigned long gfn, int slot) { xc_interface *xch = paging->xc_handle; void *page; @@ -593,7 +601,7 @@ static int xenpaging_evict_page(struct x } /* Copy page */ - ret = write_page(fd, page, slot); + ret = write_page(paging->fd, page, slot); if ( ret < 0 ) { PERROR("Error copying page %lx", gfn); @@ -670,7 +678,7 @@ static int xenpaging_resume_page(struct return ret; } -static int xenpaging_populate_page(struct xenpaging *paging, unsigned long gfn, int fd, int i) +static int xenpaging_populate_page(struct xenpaging *paging, unsigned long gfn, int i) { xc_interface *xch = paging->xc_handle; int ret; @@ -679,7 +687,7 @@ static int xenpaging_populate_page(struc DPRINTF("populate_page < gfn %lx pageslot %d\n", gfn, i); /* Read page */ - ret = read_page(fd, paging_buffer, i); + ret = read_page(paging->fd, paging_buffer, i); if ( ret != 0 ) { PERROR("Error reading page"); @@ -738,7 +746,7 @@ static void resume_pages(struct xenpagin * Returns 0 on successful evict * Returns > 0 if no gfn can be evicted */ -static int evict_victim(struct xenpaging *paging, int fd, int slot) +static int evict_victim(struct xenpaging *paging, int slot) { xc_interface *xch = paging->xc_handle; unsigned long gfn; @@ -771,7 +779,7 @@ static int evict_victim(struct xenpaging goto out; } - ret = xenpaging_evict_page(paging, gfn, fd, slot); + ret = xenpaging_evict_page(paging, gfn, slot); if ( ret < 0 ) goto out; } @@ -791,7 +799,7 @@ static int evict_victim(struct xenpaging * Returns 0 if no gfn can be evicted * Returns > 0 on successful evict */ -static int evict_pages(struct xenpaging *paging, int fd, int num_pages) +static int evict_pages(struct xenpaging *paging, int num_pages) { xc_interface *xch = paging->xc_handle; int rc, slot, num = 0; @@ -802,7 +810,7 @@ static int evict_pages(struct xenpaging if ( paging->slot_to_gfn[slot] ) continue; - rc = evict_victim(paging, fd, slot); + rc = evict_victim(paging, slot); if ( rc ) { num = rc < 0 ? -1 : num; @@ -826,10 +834,6 @@ int main(int argc, char *argv[]) int rc; xc_interface *xch; - int open_flags = O_CREAT | O_TRUNC | O_RDWR; - mode_t open_mode = S_IRUSR | S_IWUSR; - int fd; - /* Initialise domain paging */ paging = xenpaging_init(argc, argv); if ( paging == NULL ) @@ -841,14 +845,6 @@ int main(int argc, char *argv[]) DPRINTF("starting %s for domain_id %u with pagefile %s\n", argv[0], paging->mem_event.domain_id, filename); - /* Open file */ - fd = open(filename, open_flags, open_mode); - if ( fd < 0 ) - { - perror("failed to open file"); - return 2; - } - /* ensure that if we get a signal, we''ll do cleanup, then exit */ act.sa_handler = close_handler; act.sa_flags = 0; @@ -911,7 +907,7 @@ int main(int argc, char *argv[]) else { /* Populate the page */ - if ( xenpaging_populate_page(paging, req.gfn, fd, slot) < 0 ) + if ( xenpaging_populate_page(paging, req.gfn, slot) < 0 ) { ERROR("Error populating page %"PRIx64"", req.gfn); goto out; @@ -1005,7 +1001,7 @@ int main(int argc, char *argv[]) paging->use_poll_timeout = 0; num = 42; } - if ( evict_pages(paging, fd, num) < 0 ) + if ( evict_pages(paging, num) < 0 ) goto out; } /* Resume some pages if target not reached */ @@ -1033,7 +1029,7 @@ int main(int argc, char *argv[]) DPRINTF("xenpaging got signal %d\n", interrupted); out: - close(fd); + close(paging->fd); unlink_pagefile(); /* Tear down domain paging */ diff -r 649791966e1a -r 4c494f4b99f4 tools/xenpaging/xenpaging.h --- a/tools/xenpaging/xenpaging.h +++ b/tools/xenpaging/xenpaging.h @@ -50,6 +50,7 @@ struct xenpaging { int *gfn_to_slot; struct mem_event mem_event; + int fd; /* number of pages for which data structures were allocated */ int max_pages; int num_paged_out;
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 09 of 10] xenpaging: move page_buffer into struct xenpaging
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937803 -3600 # Node ID 3160254dd71365b9e21a47c7253776c9f95bb29c # Parent 4c494f4b99f4da449fe72cd6ba6398e666231d9b xenpaging: move page_buffer into struct xenpaging Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 4c494f4b99f4 -r 3160254dd713 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -44,7 +44,6 @@ static char *dom_path; static char watch_token[16]; static char *filename; static int interrupted; -static void *paging_buffer = NULL; static void unlink_pagefile(void) { @@ -441,8 +440,8 @@ static struct xenpaging *xenpaging_init( goto err; } - paging_buffer = init_page(); - if ( !paging_buffer ) + paging->paging_buffer = init_page(); + if ( !paging->paging_buffer ) { PERROR("Creating page aligned load buffer"); goto err; @@ -465,6 +464,11 @@ static struct xenpaging *xenpaging_init( xs_close(paging->xs_handle); if ( xch ) xc_interface_close(xch); + if ( paging->paging_buffer ) + { + munlock(paging->paging_buffer, PAGE_SIZE); + free(paging->paging_buffer); + } if ( paging->mem_event.shared_page ) { munlock(paging->mem_event.shared_page, PAGE_SIZE); @@ -687,7 +691,7 @@ static int xenpaging_populate_page(struc DPRINTF("populate_page < gfn %lx pageslot %d\n", gfn, i); /* Read page */ - ret = read_page(paging->fd, paging_buffer, i); + ret = read_page(paging->fd, paging->paging_buffer, i); if ( ret != 0 ) { PERROR("Error reading page"); @@ -697,8 +701,7 @@ static int xenpaging_populate_page(struc do { /* Tell Xen to allocate a page for the domain */ - ret = xc_mem_paging_load(xch, paging->mem_event.domain_id, gfn, - paging_buffer); + ret = xc_mem_paging_load(xch, paging->mem_event.domain_id, gfn, paging->paging_buffer); if ( ret < 0 ) { if ( errno == ENOMEM ) diff -r 4c494f4b99f4 -r 3160254dd713 tools/xenpaging/xenpaging.h --- a/tools/xenpaging/xenpaging.h +++ b/tools/xenpaging/xenpaging.h @@ -49,6 +49,8 @@ struct xenpaging { unsigned long *slot_to_gfn; int *gfn_to_slot; + void *paging_buffer; + struct mem_event mem_event; int fd; /* number of pages for which data structures were allocated */
Olaf Hering
2012-Jan-30 15:59 UTC
[PATCH 10 of 10] xenpaging: implement stack of free slots in pagefile
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1327937804 -3600 # Node ID 9cae410331c3d63bb82e781a5d5475e1135d4d32 # Parent 3160254dd71365b9e21a47c7253776c9f95bb29c xenpaging: implement stack of free slots in pagefile Scanning the slot_to_gfn[] array for a free slot is expensive because evict_pages() always needs to scan the whole array. Remember the last slots freed during page-in requests and reuse them in evict_pages(). Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 3160254dd713 -r 9cae410331c3 tools/xenpaging/xenpaging.c --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -432,6 +432,11 @@ static struct xenpaging *xenpaging_init( if ( !paging->slot_to_gfn || !paging->gfn_to_slot ) goto err; + /* Allocate stack for known free slots in pagefile */ + paging->free_slot_stack = calloc(paging->max_pages, sizeof(*paging->free_slot_stack)); + if ( !paging->free_slot_stack ) + goto err; + /* Initialise policy */ rc = policy_init(paging); if ( rc != 0 ) @@ -483,6 +488,7 @@ static struct xenpaging *xenpaging_init( free(dom_path); free(watch_target_tot_pages); + free(paging->free_slot_stack); free(paging->slot_to_gfn); free(paging->gfn_to_slot); free(paging->bitmap); @@ -807,6 +813,20 @@ static int evict_pages(struct xenpaging xc_interface *xch = paging->xc_handle; int rc, slot, num = 0; + /* Reuse known free slots */ + while ( paging->stack_count > 0 && num < num_pages ) + { + slot = paging->free_slot_stack[--paging->stack_count]; + rc = evict_victim(paging, slot); + if ( rc ) + { + num = rc < 0 ? -1 : num; + return num; + } + num++; + } + + /* Scan all slots slots for remainders */ for ( slot = 0; slot < paging->max_pages && num < num_pages; slot++ ) { /* Slot is allocated */ @@ -930,6 +950,9 @@ int main(int argc, char *argv[]) /* Clear this pagefile slot */ paging->slot_to_gfn[slot] = 0; + + /* Record this free slot */ + paging->free_slot_stack[paging->stack_count++] = slot; } else { diff -r 3160254dd713 -r 9cae410331c3 tools/xenpaging/xenpaging.h --- a/tools/xenpaging/xenpaging.h +++ b/tools/xenpaging/xenpaging.h @@ -60,6 +60,8 @@ struct xenpaging { int policy_mru_size; int use_poll_timeout; int debug; + int stack_count; + int *free_slot_stack; unsigned long pagein_queue[XENPAGING_PAGEIN_QUEUE_SIZE]; };
Ian Jackson
2012-Feb-09 17:57 UTC
Re: [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements
Olaf Hering writes ("[Xen-devel] [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements"):> This series adjusts the error reporting in the various code paths, with > the intention that fatal errors can be detected by callers and handled > properly. During my performance analysis with callgrind I found and > fixed a few bottlenecks in the page-in code paths.Since these are all xenpaging changes, I''m inclined to follow your lead and commit them soon. If anyone has any comments, please shout! Thanks, Ian.
Olaf Hering
2012-Feb-14 21:08 UTC
Re: [PATCH 06 of 10] xenpaging: improve performance in policy_choose_victim
On Mon, Jan 30, Olaf Hering wrote:> +++ b/tools/xenpaging/policy_default.c > @@ -80,33 +80,58 @@ int policy_init(struct xenpaging *paging > unsigned long policy_choose_victim(struct xenpaging *paging)> + > + /* gfn busy */ > + if ( test_bit(current_gfn, bitmap) ) > + continue; > + > + /* gfn already tested */ > + if ( test_bit(current_gfn, bitmap) ) > + continue; > + > + /* gfn found */ > + break;The second one should be "unconsumed" instead of "bitmap", otherwise wraparounds lead to endless loop in caller.. Olaf
Ian Jackson
2012-Feb-20 17:25 UTC
Re: [PATCH 06 of 10] xenpaging: improve performance in policy_choose_victim [and 1 more messages]
Ian Jackson writes ("Re: [Xen-devel] [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements"):> Olaf Hering writes ("[Xen-devel] [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements"): > > This series adjusts the error reporting in the various code paths, with > > the intention that fatal errors can be detected by callers and handled > > properly. During my performance analysis with callgrind I found and > > fixed a few bottlenecks in the page-in code paths. > > Since these are all xenpaging changes, I''m inclined to follow your > lead and commit them soon. If anyone has any comments, please shout!Olaf, can you please rebase to current tip ? The other recent xenpaging changes conflict with these. And then while you''re there you should obviously incorporate this change: Olaf Hering writes ("Re: [Xen-devel] [PATCH 06 of 10] xenpaging: improve performance in policy_choose_victim"):> On Mon, Jan 30, Olaf Hering wrote: > > + if ( test_bit(current_gfn, bitmap) ) > > + continue; > > + > > + /* gfn already tested */ > > + if ( test_bit(current_gfn, bitmap) ) > > + continue; > > + > > + /* gfn found */ > > + break; > > The second one should be "unconsumed" instead of "bitmap", otherwise > wraparounds lead to endless loop in caller..Thanks, Ian.
Olaf Hering
2012-Feb-20 18:48 UTC
Re: [PATCH 06 of 10] xenpaging: improve performance in policy_choose_victim [and 1 more messages]
On Mon, Feb 20, Ian Jackson wrote:> Ian Jackson writes ("Re: [Xen-devel] [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements"): > > Olaf Hering writes ("[Xen-devel] [PATCH 00 of 10] tools/xenpaging: cleanups and performance improvements"): > > > This series adjusts the error reporting in the various code paths, with > > > the intention that fatal errors can be detected by callers and handled > > > properly. During my performance analysis with callgrind I found and > > > fixed a few bottlenecks in the page-in code paths. > > > > Since these are all xenpaging changes, I''m inclined to follow your > > lead and commit them soon. If anyone has any comments, please shout! > > Olaf, can you please rebase to current tip ? The other recent > xenpaging changes conflict with these.I will send a rebased version of this series. Olaf