[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH] mem_sharing: fix race condition of nominate and unshare
At 16:11 +0000 on 06 Jan (1294330319), Jui-Hao Chiang wrote: > Hi, this patch does the following > (1) When updating/checking p2m type for mem_sharing, we must hold shr_lock > (2) For nominate operation, if the page is already nominated, return the > handle from page_info->shr_handle > (3) For unshare operation, it is possible that multiple users unshare a page > via hvm_hap_nested_page_fault() at the same time. If the page is already > un-shared by someone else, simply return success. I'm going to apply this, since it looks like an improvement, but I'm not convinced it properly solves the problem. > NOTE: we assume that nobody holds page_alloc_lock/p2m_lock before calling > nominate/share/unshare. As I told you earlier, that's not the case. p2m_teardown() can call mem_sharing_unshare_page() with the p2m lock held. Cheers, Tim. > Signed-off-by: Jui-Hao Chiang > <juihaochiang@xxxxxxxxx<mailto:juihaochiang@xxxxxxxxx>> > Signed-off-by: Han-Lin Li <Han-Lin.Li@xxxxxxxxxxx<mailto:Li@xxxxxxxxxxx>> > > Bests, > Jui-Hao > diff -r 7b4c82f07281 xen/arch/x86/mm/mem_sharing.c > --- a/xen/arch/x86/mm/mem_sharing.c Wed Jan 05 23:54:15 2011 +0000 > +++ b/xen/arch/x86/mm/mem_sharing.c Thu Jan 06 23:46:28 2011 +0800 > @@ -502,6 +502,7 @@ int mem_sharing_nominate_page(struct p2m > > *phandle = 0UL; > > + shr_lock(); > mfn = gfn_to_mfn(p2m, gfn, &p2mt); > > /* Check if mfn is valid */ > @@ -509,29 +510,33 @@ int mem_sharing_nominate_page(struct p2m > if (!mfn_valid(mfn)) > goto out; > > + /* Return the handle if the page is already shared */ > + page = mfn_to_page(mfn); > + if (p2m_is_shared(p2mt)) { > + *phandle = page->shr_handle; > + ret = 0; > + goto out; > + } > + > /* Check p2m type */ > if (!p2m_is_sharable(p2mt)) > goto out; > > /* Try to convert the mfn to the sharable type */ > - page = mfn_to_page(mfn); > ret = page_make_sharable(d, page, expected_refcnt); > if(ret) > goto out; > > /* Create the handle */ > ret = -ENOMEM; > - shr_lock(); > handle = next_handle++; > if((hash_entry = mem_sharing_hash_insert(handle, mfn)) == NULL) > { > - shr_unlock(); > goto out; > } > if((gfn_info = mem_sharing_gfn_alloc()) == NULL) > { > mem_sharing_hash_destroy(hash_entry); > - shr_unlock(); > goto out; > } > > @@ -545,7 +550,6 @@ int mem_sharing_nominate_page(struct p2m > BUG_ON(page_make_private(d, page) != 0); > mem_sharing_hash_destroy(hash_entry); > mem_sharing_gfn_destroy(gfn_info, 0); > - shr_unlock(); > goto out; > } > > @@ -559,11 +563,11 @@ int mem_sharing_nominate_page(struct p2m > gfn_info->domain = d->domain_id; > page->shr_handle = handle; > *phandle = handle; > - shr_unlock(); > > ret = 0; > > out: > + shr_unlock(); > return ret; > } > > @@ -633,14 +637,21 @@ int mem_sharing_unshare_page(struct p2m_ > struct list_head *le; > struct domain *d = p2m->domain; > > + mem_sharing_audit(); > + /* Remove the gfn_info from the list */ > + shr_lock(); > + > mfn = gfn_to_mfn(p2m, gfn, &p2mt); > + > + /* Has someone already unshared it? */ > + if (!p2m_is_shared(p2mt)) { > + shr_unlock(); > + return 0; > + } > > page = mfn_to_page(mfn); > handle = page->shr_handle; > > - mem_sharing_audit(); > - /* Remove the gfn_info from the list */ > - shr_lock(); > hash_entry = mem_sharing_hash_lookup(handle); > list_for_each(le, &hash_entry->gfns) > { > @@ -707,7 +718,6 @@ private_page_found: > mem_sharing_hash_delete(handle); > else > atomic_dec(&nr_saved_mfns); > - shr_unlock(); > > if(p2m_change_type(p2m, gfn, p2m_ram_shared, p2m_ram_rw) != > p2m_ram_shared) > @@ -718,6 +728,7 @@ private_page_found: > /* Update m2p entry */ > set_gpfn_from_mfn(mfn_x(page_to_mfn(page)), gfn); > > + shr_unlock(); > return 0; > } > -- Tim Deegan <Tim.Deegan@xxxxxxxxxx> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |