xenpaging fixes for xen-unstable

PATCH In review Olaf Hering 3 participants 7 messages Oct 3, 2011 – Oct 6, 2011

Share & tools

Olaf Hering

author

2011-10-03 14:43 UTC #0 lore raw

PATCH 1/1 xenpaging fixes for xen-unstable

The following series changes the p2m_mem_paging* functions to modify the p2mt under the p2m_lock, and there is a change to improve PoD handling.

Please review and apply.

Olaf

 xen/arch/x86/mm/p2m-pod.c |   15 ++++++++++
 xen/arch/x86/mm/p2m.c     |   69 ++++++++++++++++++++++++++++------------------
 2 files changed, 57 insertions(+), 27 deletions(-)

Olaf Hering

author

2011-10-03 14:43 UTC #1 lore raw

PATCH 1/1 xenpaging: use p2m->get_entry() in p2m_mem_paging functions

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1317652809 -7200
# Node ID a96c307da5101c610fed1ca5fe877f220071d29e # Parent e78cd03b0308c3ba5737ba9821bf7272f45549ca
xenpaging: use p2m->get_entry() in p2m_mem_paging functions

Use p2m->get_entry() in the p2m_mem_paging functions. This preserves the p2m_access type when gfn is updated with set_p2m_entry(). Its also a preparation for locking fixes in a subsequent patch.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/x86/mm/p2m.c +16 ~~-10~~

diff -r e78cd03b0308 -r a96c307da510 xen/arch/x86/mm/p2m.c
@@ -671,10 +671,11 @@ int p2m_mem_paging_nominate(struct domai
     struct page_info *page;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     p2m_type_t p2mt;
+    p2m_access_t a;
     mfn_t mfn;
     int ret;
 
-    mfn = gfn_to_mfn(p2m->domain, gfn, &p2mt);
+    mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
 
     /* Check if mfn is valid */
     ret = -EINVAL;

@@ -701,7 +702,7 @@ int p2m_mem_paging_nominate(struct domai
 
     /* Fix p2m entry */
     p2m_lock(p2m);
-    set_p2m_entry(p2m, gfn, mfn, 0, p2m_ram_paging_out, p2m->default_access);
+    set_p2m_entry(p2m, gfn, mfn, 0, p2m_ram_paging_out, a);
     audit_p2m(p2m, 1);
     p2m_unlock(p2m);

@@ -715,11 +716,12 @@ int p2m_mem_paging_evict(struct domain *
 {
     struct page_info *page;
     p2m_type_t p2mt;
+    p2m_access_t a;
     mfn_t mfn;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
     /* Get mfn */
-    mfn = gfn_to_mfn(d, gfn, &p2mt);
+    mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     if ( unlikely(!mfn_valid(mfn)) )
         return -EINVAL;

@@ -738,8 +740,7 @@ int p2m_mem_paging_evict(struct domain *
 
     /* Remove mapping from p2m table */
     p2m_lock(p2m);
-    set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), 0, 
-                  p2m_ram_paged, p2m->default_access);
+    set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), 0, p2m_ram_paged, a);
     audit_p2m(p2m, 1);
     p2m_unlock(p2m);

@@ -775,6 +776,7 @@ void p2m_mem_paging_populate(struct doma
     struct vcpu *v = current;
     mem_event_request_t req;
     p2m_type_t p2mt;
+    p2m_access_t a;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
     /* Check that there's space on the ring for this request */

@@ -787,12 +789,12 @@ void p2m_mem_paging_populate(struct doma
     /* Fix p2m mapping */
     /* XXX: It seems inefficient to have this here, as it's only needed
      *      in one case (ept guest accessing paging out page) */
-    gfn_to_mfn(d, gfn, &p2mt);
+    p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     if ( p2mt == p2m_ram_paged )
     {
         p2m_lock(p2m);
         set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), 0, 
-                      p2m_ram_paging_in_start, p2m->default_access);
+                      p2m_ram_paging_in_start, a);
         audit_p2m(p2m, 1);
         p2m_unlock(p2m);
     }

@@ -821,8 +823,11 @@ void p2m_mem_paging_populate(struct doma
 int p2m_mem_paging_prep(struct domain *d, unsigned long gfn)
 {
     struct page_info *page;
+    p2m_type_t p2mt;
+    p2m_access_t a;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
+    p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     /* Get a free page */
     page = alloc_domheap_page(p2m->domain, 0);
     if ( unlikely(page == NULL) )

@@ -830,7 +835,7 @@ int p2m_mem_paging_prep(struct domain *d
 
     /* Fix p2m mapping */
     p2m_lock(p2m);
-    set_p2m_entry(p2m, gfn, page_to_mfn(page), 0, p2m_ram_paging_in, p2m->default_access);
+    set_p2m_entry(p2m, gfn, page_to_mfn(page), 0, p2m_ram_paging_in, a);
     audit_p2m(p2m, 1);
     p2m_unlock(p2m);

@@ -844,6 +849,7 @@ void p2m_mem_paging_resume(struct domain
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     mem_event_response_t rsp;
     p2m_type_t p2mt;
+    p2m_access_t a;
     mfn_t mfn;
 
     /* Pull the response off the ring */

@@ -852,9 +858,9 @@ void p2m_mem_paging_resume(struct domain
     /* Fix p2m entry if the page was not dropped */
     if ( !(rsp.flags & MEM_EVENT_FLAG_DROP_PAGE) )
     {
-        mfn = gfn_to_mfn(d, rsp.gfn, &p2mt);
+        mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query, NULL);
         p2m_lock(p2m);
-        set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, p2m->default_access);
+        set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a);
         set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn);
         audit_p2m(p2m, 1);
         p2m_unlock(p2m);

Olaf Hering

author

2011-10-03 14:43 UTC #2 lore raw

PATCH 1/1 xenpaging: fix locking in p2m_mem_paging functions

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1317652810 -7200
# Node ID 6bf1aa780b9ff40c50cfd00cfa8796ccc76286ee # Parent a96c307da5101c610fed1ca5fe877f220071d29e
xenpaging: fix locking in p2m_mem_paging functions

As suggested by <[email protected]>, query and adjust the p2mt under the p2m_lock to prevent races with PoD.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/x86/mm/p2m.c +27 ~~-16~~

diff -r a96c307da510 -r 6bf1aa780b9f xen/arch/x86/mm/p2m.c
@@ -675,6 +675,8 @@ int p2m_mem_paging_nominate(struct domai
     mfn_t mfn;
     int ret;
 
+    p2m_lock(p2m);
+
     mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
 
     /* Check if mfn is valid */

@@ -701,14 +703,12 @@ int p2m_mem_paging_nominate(struct domai
         goto out;
 
     /* Fix p2m entry */
-    p2m_lock(p2m);
     set_p2m_entry(p2m, gfn, mfn, 0, p2m_ram_paging_out, a);
     audit_p2m(p2m, 1);
-    p2m_unlock(p2m);
-
     ret = 0;
 
  out:
+    p2m_unlock(p2m);
     return ret;
 }

@@ -719,30 +719,31 @@ int p2m_mem_paging_evict(struct domain *
     p2m_access_t a;
     mfn_t mfn;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    int ret = -EINVAL;
+
+    p2m_lock(p2m);
 
     /* Get mfn */
     mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     if ( unlikely(!mfn_valid(mfn)) )
-        return -EINVAL;
+        goto out;
 
     if ( (p2mt == p2m_ram_paged) || (p2mt == p2m_ram_paging_in) ||
          (p2mt == p2m_ram_paging_in_start) )
-        return -EINVAL;
+        goto out;
 
     /* Get the page so it doesn't get modified under Xen's feet */
     page = mfn_to_page(mfn);
     if ( unlikely(!get_page(page, d)) )
-        return -EINVAL;
+        goto out;
 
     /* Decrement guest domain's ref count of the page */
     if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
     /* Remove mapping from p2m table */
-    p2m_lock(p2m);
     set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), 0, p2m_ram_paged, a);
     audit_p2m(p2m, 1);
-    p2m_unlock(p2m);
 
     /* Put the page back so it gets freed */
     put_page(page);

@@ -750,7 +751,11 @@ int p2m_mem_paging_evict(struct domain *
     /* Track number of paged gfns */
     atomic_inc(&d->paged_pages);
 
-    return 0;
+    ret = 0;
+
+ out:
+    p2m_unlock(p2m);
+    return ret;
 }
 
 void p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn)

@@ -789,15 +794,15 @@ void p2m_mem_paging_populate(struct doma
     /* Fix p2m mapping */
     /* XXX: It seems inefficient to have this here, as it's only needed
      *      in one case (ept guest accessing paging out page) */
+    p2m_lock(p2m);
     p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     if ( p2mt == p2m_ram_paged )
     {
-        p2m_lock(p2m);
         set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), 0, 
                       p2m_ram_paging_in_start, a);
         audit_p2m(p2m, 1);
-        p2m_unlock(p2m);
     }
+    p2m_unlock(p2m);
 
     /* Pause domain */
     if ( v->domain->domain_id == d->domain_id )

@@ -826,22 +831,28 @@ int p2m_mem_paging_prep(struct domain *d
     p2m_type_t p2mt;
     p2m_access_t a;
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    int ret = -ENOMEM;
+
+    p2m_lock(p2m);
 
     p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
+
     /* Get a free page */
     page = alloc_domheap_page(p2m->domain, 0);
     if ( unlikely(page == NULL) )
-        return -ENOMEM;
+        goto out;
 
     /* Fix p2m mapping */
-    p2m_lock(p2m);
     set_p2m_entry(p2m, gfn, page_to_mfn(page), 0, p2m_ram_paging_in, a);
     audit_p2m(p2m, 1);
-    p2m_unlock(p2m);
 
     atomic_dec(&d->paged_pages);
 
-    return 0;
+    ret = 0;
+
+ out:
+    p2m_unlock(p2m);
+    return ret;
 }
 
 void p2m_mem_paging_resume(struct domain *d)

@@ -858,8 +869,8 @@ void p2m_mem_paging_resume(struct domain
     /* Fix p2m entry if the page was not dropped */
     if ( !(rsp.flags & MEM_EVENT_FLAG_DROP_PAGE) )
     {
+        p2m_lock(p2m);
         mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query, NULL);
-        p2m_lock(p2m);
         set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a);
         set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn);
         audit_p2m(p2m, 1);

Olaf Hering

author

2011-10-03 14:43 UTC #3 lore raw

PATCH 1/1 xenpaging: remove confusing comment from p2m_mem_paging_populate

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1317652811 -7200
# Node ID 13872c432c3807e0f977d9c1311801179807ece2
# Parent 6bf1aa780b9ff40c50cfd00cfa8796ccc76286ee
xenpaging: remove confusing comment from p2m_mem_paging_populate

Currently there is no way to avoid the double check of the p2mt because p2m_mem_paging_populate() is called from many places without the p2m_lock held. Upcoming changes will move the function into gfn_to_mfn(), so its interface could be changed and the extra p2m_lock/get_entry can be removed.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/x86/mm/p2m.c +0 -2

diff -r 6bf1aa780b9f -r 13872c432c38 xen/arch/x86/mm/p2m.c
@@ -792,8 +792,6 @@ void p2m_mem_paging_populate(struct doma
     req.type = MEM_EVENT_TYPE_PAGING;
 
     /* Fix p2m mapping */
-    /* XXX: It seems inefficient to have this here, as it's only needed
-     *      in one case (ept guest accessing paging out page) */
     p2m_lock(p2m);
     p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     if ( p2mt == p2m_ram_paged )

Olaf Hering

author

2011-10-03 14:43 UTC #4 lore raw

PATCH 1/1 xenpaging: handle paged pages in p2m_pod_decrease_reservation

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1317652812 -7200
# Node ID b05ede64aaf5f5090fdb844c3a58f1f92d9b3588 # Parent 13872c432c3807e0f977d9c1311801179807ece2
xenpaging: handle paged pages in p2m_pod_decrease_reservation

As suggested by <[email protected]>, handle paged pages in PoD code.

Signed-off-by Olaf Hering <[email protected]>

xen/arch/x86/mm/p2m-pod.c +15 -0

diff -r 13872c432c38 -r b05ede64aaf5 xen/arch/x86/mm/p2m-pod.c
@@ -567,6 +567,21 @@ p2m_pod_decrease_reservation(struct doma
             BUG_ON(p2m->pod.entry_count < 0);
             pod--;
         }
+        else if ( steal_for_cache && p2m_is_paging(t) )
+        {
+            struct page_info *page;
+            /* alloc a new page to compensate the pod list */
+            page = alloc_domheap_page(d, 0);
+            if ( !page )
+                goto out_entry_check;
+            set_p2m_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid, p2m->default_access);
+            p2m_mem_paging_drop_page(d, gpfn+i);
+            p2m_pod_cache_add(p2m, page, 0);
+            steal_for_cache =  ( p2m->pod.entry_count > p2m->pod.count );
+            nonpod--;
+            ram--;
+        }
+        /* for other ram types */
         else if ( steal_for_cache && p2m_is_ram(t) )
         {
             struct page_info *page;

George Dunlap

2011-10-04 16:12 UTC #5 lore raw

Re: [PATCH 4 of 4] xenpaging: handle paged pages in p2m_pod_decrease_reservation

Quoted 15 lines · Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1317652812 -7200
# Node ID b05ede64aaf5f5090fdb844c3a58f1f92d9b3588 # Parent 13872c432c3807e0f977d9c1311801179807ece2
xenpaging: handle paged pages in p2m_pod_decrease_reservation
As suggested by <[email protected]>, handle paged pages in PoD code.
Signed-off-by Olaf Hering <[email protected]>
diff -r 13872c432c38 -r b05ede64aaf5 xen/arch/x86/mm/p2m-pod.c

xen/arch/x86/mm/p2m-pod.c p2m_pod_decrease_reservation(struct doma · line 573

Quoted 7 lines

xen/arch/x86/mm/p2m-pod.c +4 -0

@@ -567,6 +567,21 @@ p2m_pod_decrease_reservation(struct doma
             BUG_ON(p2m->pod.entry_count < 0);
             pod--;
         }
+        else if ( steal_for_cache && p2m_is_paging(t) )
+        {
+            struct page_info *page;
+            /* alloc a new page to compensate the pod list */

This can't be right. The whole point of the "populate on demand" was to pre-allocate a fixed amount of memory, and not need to have to allocate any more. What happens if this allocation fails?

It seems like a better thing to do might be this: If we get a request to swap out a page, and we still have PoD entries present, we "swap out" that page as a zero page.

Hmm -- this will take some careful thought...

Quoted 20 lines

xen/arch/x86/mm/p2m-pod.c +11 -0

+            page = alloc_domheap_page(d, 0);
+            if ( !page )
+                goto out_entry_check;
+            set_p2m_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid, p2m->default_access);
+            p2m_mem_paging_drop_page(d, gpfn+i);
+            p2m_pod_cache_add(p2m, page, 0);
+            steal_for_cache =  ( p2m->pod.entry_count > p2m->pod.count );
+            nonpod--;
+            ram--;
+        }
+        /* for other ram types */
         else if ( steal_for_cache && p2m_is_ram(t) )
         {
             struct page_info *page;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Tim Deegan

2011-10-06 11:48 UTC #6 lore raw

In reply to Olaf Hering

Quoted 5 lines · Olaf Hering

The following series changes the p2m_mem_paging* functions to modify the p2mt under the p2m_lock, and there is a change to improve PoD handling.
Please review and apply.

I've applied patches 1-3; leaving 4 until the relationship between paging, sharing, ballooning and PoD is properly understood.

Cheers,

Tim.