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
|
Home