[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 04 of 12] x86/mm: Enforce lock ordering for sharing page locks



 xen/arch/x86/mm/mem_sharing.c |  91 ++++++++++++++++++++++++++----------------
 xen/arch/x86/mm/mm-locks.h    |  18 ++++++++-
 xen/include/asm-x86/mm.h      |   3 +-
 3 files changed, 76 insertions(+), 36 deletions(-)


Use the ordering constructs in mm-locks.h to enforce an order
for the p2m and page locks in the sharing code. Applies to either
the global sharing lock (in audit mode) or the per page locks.

Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
Signed-off-by: Adin Scanneell <adin@xxxxxxxxxxx>

diff -r 11916fe20dd2 -r c906c616d5ac xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -37,6 +37,14 @@
 
 static shr_handle_t next_handle = 1;
 
+typedef struct pg_lock_data {
+    int mm_unlock_level;
+    unsigned short recurse_count;
+} pg_lock_data_t;
+
+#define DECLARE_PG_LOCK_DATA(name) \
+    pg_lock_data_t  name = { 0, 0 };
+
 #if MEM_SHARING_AUDIT
 
 static mm_lock_t shr_lock;
@@ -63,11 +71,12 @@ static inline void audit_del_list(struct
     list_del(&page->shared_info->entry);
 }
 
-static inline int mem_sharing_page_lock(struct page_info *p)
+static inline int mem_sharing_page_lock(struct page_info *p, 
+                                        pg_lock_data_t *l)
 {
     return 1;
 }
-#define mem_sharing_page_unlock(p)   ((void)0)
+#define mem_sharing_page_unlock(p, l)   ((void)0)
 
 #define get_next_handle()   next_handle++;
 #else
@@ -85,19 +94,26 @@ static inline int mem_sharing_audit(void
 #define audit_add_list(p)  ((void)0)
 #define audit_del_list(p)  ((void)0)
 
-static inline int mem_sharing_page_lock(struct page_info *pg)
+static inline int mem_sharing_page_lock(struct page_info *pg,
+                                        pg_lock_data_t *pld)
 {
     int rc;
+    page_sharing_mm_pre_lock();
     rc = page_lock(pg);
     if ( rc )
     {
         preempt_disable();
+        page_sharing_mm_post_lock(&pld->mm_unlock_level, 
+                                  &pld->recurse_count);
     }
     return rc;
 }
 
-static inline void mem_sharing_page_unlock(struct page_info *pg)
+static inline void mem_sharing_page_unlock(struct page_info *pg,
+                                           pg_lock_data_t *pld)
 {
+    page_sharing_mm_unlock(pld->mm_unlock_level, 
+                           &pld->recurse_count);
     preempt_enable();
     page_unlock(pg);
 }
@@ -492,7 +508,8 @@ static int page_make_sharable(struct dom
     return 0;
 }
 
-static int page_make_private(struct domain *d, struct page_info *page)
+static int page_make_private(struct domain *d, struct page_info *page,
+                             pg_lock_data_t *pld)
 {
     unsigned long expected_type;
 
@@ -520,9 +537,11 @@ static int page_make_private(struct doma
     /* Drop the final typecount */
     put_page_and_type(page);
 
-#ifndef MEM_SHARING_AUDIT
+#if MEM_SHARING_AUDIT
+    (void)pld;
+#else
     /* Now that we've dropped the type, we can unlock */
-    mem_sharing_page_unlock(page);
+    mem_sharing_page_unlock(page, pld);
 #endif
 
     /* Change the owner */
@@ -538,7 +557,8 @@ static int page_make_private(struct doma
     return 0;
 }
 
-static inline struct page_info *__grab_shared_page(mfn_t mfn)
+static inline struct page_info *__grab_shared_page(mfn_t mfn,
+                                                    pg_lock_data_t *pld)
 {
     struct page_info *pg = NULL;
 
@@ -548,12 +568,12 @@ static inline struct page_info *__grab_s
 
     /* If the page is not validated we can't lock it, and if it's  
      * not validated it's obviously not shared. */
-    if ( !mem_sharing_page_lock(pg) )
+    if ( !mem_sharing_page_lock(pg, pld) )
         return NULL;
 
     if ( mem_sharing_lookup(mfn_x(mfn)) == NULL )
     {
-        mem_sharing_page_unlock(pg);
+        mem_sharing_page_unlock(pg, pld);
         return NULL;
     }
 
@@ -570,6 +590,7 @@ int mem_sharing_nominate_page(struct dom
     struct page_info *page = NULL; /* gcc... */
     int ret;
     struct gfn_info *gfn_info;
+    DECLARE_PG_LOCK_DATA(pld);
 
     *phandle = 0UL;
 
@@ -583,7 +604,7 @@ int mem_sharing_nominate_page(struct dom
 
     /* Return the handle if the page is already shared */
     if ( p2m_is_shared(p2mt) ) {
-        struct page_info *pg = __grab_shared_page(mfn);
+        struct page_info *pg = __grab_shared_page(mfn, &pld);
         if ( !pg )
         {
             gdprintk(XENLOG_ERR, "Shared p2m entry gfn %lx, but could not "
@@ -592,7 +613,7 @@ int mem_sharing_nominate_page(struct dom
         }
         *phandle = pg->shared_info->handle;
         ret = 0;
-        mem_sharing_page_unlock(pg);
+        mem_sharing_page_unlock(pg, &pld);
         goto out;
     }
 
@@ -610,7 +631,7 @@ int mem_sharing_nominate_page(struct dom
      * race because we're holding the p2m entry, so no one else 
      * could be nominating this gfn */
     ret = -ENOENT;
-    if ( !mem_sharing_page_lock(page) )
+    if ( !mem_sharing_page_lock(page, &pld) )
         goto out;
 
     /* Initialize the shared state */
@@ -619,7 +640,7 @@ int mem_sharing_nominate_page(struct dom
             xmalloc(struct page_sharing_info)) == NULL )
     {
         /* Making a page private atomically unlocks it */
-        BUG_ON(page_make_private(d, page) != 0);
+        BUG_ON(page_make_private(d, page, &pld) != 0);
         goto out;
     }
     page->shared_info->pg = page;
@@ -633,7 +654,7 @@ int mem_sharing_nominate_page(struct dom
     {
         xfree(page->shared_info);
         page->shared_info = NULL;
-        BUG_ON(page_make_private(d, page) != 0);
+        BUG_ON(page_make_private(d, page, &pld) != 0);
         goto out;
     }
 
@@ -648,7 +669,7 @@ int mem_sharing_nominate_page(struct dom
         xfree(page->shared_info);
         page->shared_info = NULL;
         /* NOTE: We haven't yet added this to the audit list. */
-        BUG_ON(page_make_private(d, page) != 0);
+        BUG_ON(page_make_private(d, page, &pld) != 0);
         goto out;
     }
 
@@ -657,7 +678,7 @@ int mem_sharing_nominate_page(struct dom
 
     *phandle = page->shared_info->handle;
     audit_add_list(page);
-    mem_sharing_page_unlock(page);
+    mem_sharing_page_unlock(page, &pld);
     ret = 0;
 
 out:
@@ -676,6 +697,7 @@ int mem_sharing_share_pages(struct domai
     int ret = -EINVAL;
     mfn_t smfn, cmfn;
     p2m_type_t smfn_type, cmfn_type;
+    DECLARE_PG_LOCK_DATA(pld);
 
     shr_lock();
 
@@ -699,28 +721,28 @@ int mem_sharing_share_pages(struct domai
     else if ( mfn_x(smfn) < mfn_x(cmfn) )
     {
         ret = XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID;
-        spage = firstpg = __grab_shared_page(smfn);
+        spage = firstpg = __grab_shared_page(smfn, &pld);
         if ( spage == NULL )
             goto err_out;
 
         ret = XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID;
-        cpage = secondpg = __grab_shared_page(cmfn);
+        cpage = secondpg = __grab_shared_page(cmfn, &pld);
         if ( cpage == NULL )
         {
-            mem_sharing_page_unlock(spage);
+            mem_sharing_page_unlock(spage, &pld);
             goto err_out;
         }
     } else {
         ret = XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID;
-        cpage = firstpg = __grab_shared_page(cmfn);
+        cpage = firstpg = __grab_shared_page(cmfn, &pld);
         if ( cpage == NULL )
             goto err_out;
 
         ret = XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID;
-        spage = secondpg = __grab_shared_page(smfn);
+        spage = secondpg = __grab_shared_page(smfn, &pld);
         if ( spage == NULL )
         {
-            mem_sharing_page_unlock(cpage);
+            mem_sharing_page_unlock(cpage, &pld);
             goto err_out;
         }
     }
@@ -732,15 +754,15 @@ int mem_sharing_share_pages(struct domai
     if ( spage->shared_info->handle != sh )
     {
         ret = XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID;
-        mem_sharing_page_unlock(secondpg);
-        mem_sharing_page_unlock(firstpg);
+        mem_sharing_page_unlock(secondpg, &pld);
+        mem_sharing_page_unlock(firstpg, &pld);
         goto err_out;
     }
     if ( cpage->shared_info->handle != ch )
     {
         ret = XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID;
-        mem_sharing_page_unlock(secondpg);
-        mem_sharing_page_unlock(firstpg);
+        mem_sharing_page_unlock(secondpg, &pld);
+        mem_sharing_page_unlock(firstpg, &pld);
         goto err_out;
     }
 
@@ -767,8 +789,8 @@ int mem_sharing_share_pages(struct domai
     xfree(cpage->shared_info);
     cpage->shared_info = NULL;
 
-    mem_sharing_page_unlock(secondpg);
-    mem_sharing_page_unlock(firstpg);
+    mem_sharing_page_unlock(secondpg, &pld);
+    mem_sharing_page_unlock(firstpg, &pld);
 
     /* Free the client page */
     if(test_and_clear_bit(_PGC_allocated, &cpage->count_info))
@@ -796,6 +818,7 @@ int mem_sharing_unshare_page(struct doma
     int last_gfn;
     gfn_info_t *gfn_info = NULL;
     struct list_head *le;
+    DECLARE_PG_LOCK_DATA(pld);
    
     /* This is one of the reasons why we can't enforce ordering
      * between shr_lock and p2m fine-grained locks in mm-lock. 
@@ -811,7 +834,7 @@ int mem_sharing_unshare_page(struct doma
         return 0;
     }
 
-    page = __grab_shared_page(mfn);
+    page = __grab_shared_page(mfn, &pld);
     if ( page == NULL )
     {
         gdprintk(XENLOG_ERR, "Domain p2m is shared, but page is not: "
@@ -848,7 +871,7 @@ gfn_found:
     if ( flags & MEM_SHARING_DESTROY_GFN )
     {
         put_page_and_type(page);
-        mem_sharing_page_unlock(page);
+        mem_sharing_page_unlock(page, &pld);
         if ( last_gfn && 
             test_and_clear_bit(_PGC_allocated, &page->count_info) ) 
             put_page(page);
@@ -861,7 +884,7 @@ gfn_found:
     if ( last_gfn )
     {
         /* Making a page private atomically unlocks it */
-        BUG_ON(page_make_private(d, page) != 0);
+        BUG_ON(page_make_private(d, page, &pld) != 0);
         goto private_page_found;
     }
 
@@ -869,7 +892,7 @@ gfn_found:
     page = alloc_domheap_page(d, 0);
     if ( !page ) 
     {
-        mem_sharing_page_unlock(old_page);
+        mem_sharing_page_unlock(old_page, &pld);
         put_gfn(d, gfn);
         mem_sharing_notify_helper(d, gfn);
         shr_unlock();
@@ -883,7 +906,7 @@ gfn_found:
     unmap_domain_page(t);
 
     BUG_ON(set_shared_p2m_entry(d, gfn, page_to_mfn(page)) == 0);
-    mem_sharing_page_unlock(old_page);
+    mem_sharing_page_unlock(old_page, &pld);
     put_page_and_type(old_page);
 
 private_page_found:    
diff -r 11916fe20dd2 -r c906c616d5ac xen/arch/x86/mm/mm-locks.h
--- a/xen/arch/x86/mm/mm-locks.h
+++ b/xen/arch/x86/mm/mm-locks.h
@@ -156,7 +156,23 @@ declare_mm_lock(shr)
 
 #else
 
-/* We use an efficient per-page lock when AUDIT is not enabled. */
+/* Sharing per page lock
+ *
+ * This is an external lock, not represented by an mm_lock_t. The memory
+ * sharing lock uses it to protect addition and removal of (gfn,domain)
+ * tuples to a shared page. We enforce order here against the p2m lock,
+ * which is taken after the page_lock to change the gfn's p2m entry.
+ *
+ * Note that in sharing audit mode, we use the global page lock above, 
+ * instead.
+ *
+ * The lock is recursive because during share we lock two pages. */
+
+declare_mm_order_constraint(per_page_sharing)
+#define page_sharing_mm_pre_lock()   
mm_enforce_order_lock_pre_per_page_sharing()
+#define page_sharing_mm_post_lock(l, r) \
+        mm_enforce_order_lock_post_per_page_sharing((l), (r))
+#define page_sharing_mm_unlock(l, r) mm_enforce_order_unlock((l), (r))
 
 #endif /* MEM_SHARING_AUDIT */
 
diff -r 11916fe20dd2 -r c906c616d5ac xen/include/asm-x86/mm.h
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -351,7 +351,8 @@ void clear_superpage_mark(struct page_in
  * backing. Nesting may happen when sharing (and locking) two pages -- 
deadlock 
  * is avoided by locking pages in increasing order.
  * Memory sharing may take the p2m_lock within a page_lock/unlock
- * critical section. 
+ * critical section. We enforce ordering between page_lock and p2m_lock using 
an
+ * mm-locks.h construct. 
  *
  * These two users (pte serialization and memory sharing) do not collide, since
  * sharing is only supported for hvm guests, which do not perform pv pte 
updates.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.