On Thu, Apr 05, 2018 at 22:13:01 -0400, Emilio G. Cota wrote: > +/* > + * Lock a range of pages ([@start,@end[) as well as the pages of all > + * intersecting TBs. > + * Locking order: acquire locks in ascending order of page index. > + */ > +struct page_collection * > +page_collection_lock(tb_page_addr_t start, tb_page_addr_t end) > +{ > + struct page_collection *set = g_malloc(sizeof(*set)); > + tb_page_addr_t index; > + PageDesc *pd; > + > + start >>= TARGET_PAGE_BITS; > + end >>= TARGET_PAGE_BITS; > + g_assert(start <= end); > + > + set->tree = g_tree_new_full(tb_page_addr_cmp, NULL, NULL, > + page_entry_destroy); > + set->max = NULL; > + > + retry: > + g_tree_foreach(set->tree, page_entry_lock, NULL); > + > + for (index = start; index <= end; index++) { > + TranslationBlock *tb; > + int n; > + > + pd = page_find(index); > + if (pd == NULL) { > + continue; > + }
Just noticed there's a bug here: we need to acquire pd's page lock, since it protects the traversal of the list of pd's TBs. Fixed in v3 as shown below. > + PAGE_FOR_EACH_TB(pd, tb, n) { > + if (page_trylock_add(set, tb->page_addr[0]) || > + (tb->page_addr[1] != -1 && > + page_trylock_add(set, tb->page_addr[1]))) { > + /* drop all locks, and reacquire in order */ > + g_tree_foreach(set->tree, page_entry_unlock, NULL); > + goto retry; > + } > + } > + } > + return set; > +} Thanks, Emilio --- @@ -840,6 +840,12 @@ page_collection_lock(tb_page_addr_t start, tb_page_addr_t end) if (pd == NULL) { continue; } + if (page_trylock_add(set, index << TARGET_PAGE_BITS)) { + g_tree_foreach(set->tree, page_entry_unlock, NULL); + set_page_collection_locked(false); + goto retry; + } + assert_page_locked(pd); PAGE_FOR_EACH_TB(pd, tb, n) { if (page_trylock_add(set, tb->page_addr[0]) || (tb->page_addr[1] != -1 &&