WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH] mem_sharing: fix race condition of nominate and

To: tinnycloud <tinnycloud@xxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] mem_sharing: fix race condition of nominate and unshare
From: Jui-Hao Chiang <juihaochiang@xxxxxxxxx>
Date: Mon, 10 Jan 2011 16:10:03 +0800
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Delivery-date: Mon, 10 Jan 2011 00:11:08 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type; bh=KcLJ9G/5K46wtYHqRmfXZP3eEHF+gBTzSJO4EmJlfLM=; b=IoJC8EIyA0Hk5Q/2J/ALJc38k8NQjNvUv3yQDY36RAotuHmyZU+jET/fhNoSfy22jk mpbGLK0nGQ8umxnBUYTHRGsZyB0pH4432bG81+nxpuAQZEFGillbSN+J/dMQHIhOxC96 8QFzLcMBwt02XpuqG3l76ADPI0+hrT1ArYQzw=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=sz2e/SapjNLlVORU+tyi11VyIXpdZS6E/cSaWDWIEydHv9FLfiItQV+Smo6VKzr4j2 YOehtQKkAdCcEIrCgn2StWoqVeywAcZ0GcMxdBUCKRfIupglWIryhR1xnrcSw2QXWfyP nGJ+XjCCne2PNQohOLni5qMbPOGcNNNzjf5V8=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <BLU157-ds1E01DEBE18840E5FBB6D1DA0E0@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <AANLkTinMp1v1zex2BfcUuszotPuxJFWZQNUp40gu_gxL@xxxxxxxxxxxxxx> <20110106165450.GO21948@xxxxxxxxxxxxxxxxxxxxxxx> <AANLkTinmpiusLqegGZA+bZWpDXPM+7Wq2nt8MZa0Ocet@xxxxxxxxxxxxxx> <AANLkTinf-A_4NEPQeCw0pftM5Bks8BYPRhMx3-stTHxa@xxxxxxxxxxxxxx> <BLU157-ds1E01DEBE18840E5FBB6D1DA0E0@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi, tinnycloud:

Thanks for your testing info.
I assume you have mem_sharing_unshare_page called with successful return value, otherwise the mod_l1_entry won't be called.
Could you call mem_sharing_debug_gfn() before unshare() return success?
In addition, are there multiple CPUs touching the same page? e.g. you can print out the cpu id inside unshare() and the mm.c:859.


After this change, unshare() has a potential problem of deadlock for shr_lock and p2m_lock with different locking order.
Assume two CPUs do the following
CPU1: hvm_hap_nested_page_fault() => unshare() => p2m_change_type() (locking order: shr_lock, p2m_lock)
CPU2: p2m_teardown() => unshare() (locking order: p2m_lock, shr_lock)
When CPU1 grabs shr_lock and CPU2 grabs p2m_lock, they deadlock later.

 1.       mem_sharing_unshare_page() has the routine  called from gfn_to_mfn_unshare, which is called by gnttab_transfer  

Since no bug report on grant_table right now, so I think this is safe for now

Also  p2m_tear_down è mem_sharing_unshare_page() , its flag is MEM_SHARING_DESTROY_GFN, and won’t has the chance to

call set_shared_p2m_entry()

 

Of course, the p2m_teardown won't call set_shared_p2m_entry. But this does not change my argument that p2m_teardown() hold p2m_lock to wait on shr_lock. Actaully, after looking for a while, I rebut myself that the scenario of deadlock won't exist.
When p2m_teardown is called, the domain is dying in its last few steps (device, irq are released), and there is no way for hvm_hap_nested_page_fault() to happen on the memory of the dying domain. If this case is eliminated, then my patch should not have deadlock problem. Any comments?

3.       set_shared_p2m_entry() which call set_p2m_entry() is not in p2m_lock, and I found in other code set_p2m_entry is called in p2m_lock,

so here I think it is a problem

 

So I think at least set_p2m_entry should be put into p2m_lock.

I’ll do more investigation base on this.

 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>