Skip to contentSkip to conversation

RFC: wait queue usage

PATCH In review Olaf Hering 2 participants 9 messages Dec 1, 2011 – Dec 15, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RFC: wait queue usage

The following series is my current state of an attempt to use wait queues for xenpaging and mem_event handling. I post it here for review and comments.

This series conflicts with work from Andres Lagar-Cavilla.

Olaf

 xen/arch/x86/hvm/emulate.c       |    3 
 xen/arch/x86/hvm/hvm.c           |   21 +-
 xen/arch/x86/mm.c                |   49 +----
 xen/arch/x86/mm/guest_walk.c     |    3 
 xen/arch/x86/mm/hap/guest_walk.c |    6 
 xen/arch/x86/mm/mem_event.c      |  150 +++++++++++++--
 xen/arch/x86/mm/mem_sharing.c    |   27 +-
 xen/arch/x86/mm/p2m-ept.c        |    3 
 xen/arch/x86/mm/p2m.c            |  378 +++++++++++++++++++++++++++++++++------
 xen/common/domctl.c              |    3 
 xen/common/grant_table.c         |    3 
 xen/include/asm-x86/hvm/domain.h |    3 
 xen/include/asm-x86/mem_event.h  |    8 
 xen/include/asm-x86/p2m.h        |   11 -
 xen/include/public/trace.h       |   31 +++
 xen/include/xen/sched.h          |   16 +
 16 files changed, 569 insertions(+), 146 deletions(-)
#1 lore raw

PATCH 1/1 mem_event: use wait queue when ring is full

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1322736732 -3600
# Node ID 612f69531fd15cf59c58404f6e4762733a9a268c # Parent 3c4c29899d8a2a0c0f9109b7236da95bb72b77b6
mem_event: use wait queue when ring is full

This change is based on an idea/patch from Adin Scannell.

If the ring is full, put the current vcpu to sleep if it belongs to the target domain. The wakeup happens in the p2m_*_resume functions. Wakeup will take the number of free slots into account.

A request from another domain will use the existing ->req_producers functionality because sleeping is not possible if the vcpu does not belong to the target domain.

This change fixes also a bug in p2m_mem_paging_drop_page(). Up to now a full ring will lead to harmless inconsistency in the pager.

v4:
- fix off-by-one bug in _mem_event_put_request
- add mem_event_wake_requesters() and use wake_up_nr()
- rename mem_event_mark_and_pause() and mem_event_mark_and_pause() functions
- req_producers counts foreign request producers, rename member

v3:
- rename ->mem_event_bit to ->bit
- remove me_ from new VPF_ defines

v2:
- p2m_mem_paging_populate: move vcpu_pause after put_request, otherwise the vcpu will not wake_up after a wait_event because the pause_count was increased twice. Fixes guest hangs.
- update free space check in _mem_event_put_request()
- simplify mem_event_put_request()

xen/arch/x86/hvm/hvm.c +1 -1
diff -r 3c4c29899d8a -r 612f69531fd1 xen/arch/x86/hvm/hvm.c
@@ -4107,7 +4107,7 @@ static int hvm_memory_event_traps(long p
         return 1;
     
     rc = mem_event_check_ring(d, &d->mem_event->access);
-    if ( rc )
+    if ( rc < 0 )
         return rc;
     
     memset(&req, 0, sizeof(req));
xen/arch/x86/mm/mem_event.c +117 -17

229 lines not on this page · raw message

xen/arch/x86/mm/mem_sharing.c +10 -17
diff -r 3c4c29899d8a -r 612f69531fd1 xen/arch/x86/mm/mem_sharing.c
@@ -253,19 +253,11 @@ static void mem_sharing_audit(void)
 #endif
 
 
-static struct page_info* mem_sharing_alloc_page(struct domain *d, 
-                                                unsigned long gfn)
+static void mem_sharing_notify_helper(struct domain *d, unsigned long gfn)
 {
-    struct page_info* page;
     struct vcpu *v = current;
     mem_event_request_t req;
 
-    page = alloc_domheap_page(d, 0); 
-    if(page != NULL) return page;
-
-    memset(&req, 0, sizeof(req));
-    req.type = MEM_EVENT_TYPE_SHARED;
-
     if ( v->domain != d )
     {
         /* XXX This path needs some attention.  For now, just fail foreign 
@@ -275,20 +267,20 @@ static struct page_info* mem_sharing_all
         gdprintk(XENLOG_ERR, 
                  "Failed alloc on unshare path for foreign (%d) lookup\n",
                  d->domain_id);
-        return page;
+        return;
     }
 
-    vcpu_pause_nosync(v);
-    req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
+    if (mem_event_check_ring(d, &d->mem_event->share) < 0)
+        return;
 
-    if(mem_event_check_ring(d, &d->mem_event->share)) return page;
-
+    memset(&req, 0, sizeof(req));
+    req.type = MEM_EVENT_TYPE_SHARED;
+    req.flags = MEM_EVENT_FLAG_VCPU_PAUSED;
     req.gfn = gfn;
     req.p2mt = p2m_ram_shared;
     req.vcpu_id = v->vcpu_id;
     mem_event_put_request(d, &d->mem_event->share, &req);
-
-    return page;
+    vcpu_pause_nosync(v);
 }
 
 unsigned int mem_sharing_get_nr_saved_mfns(void)
@@ -653,7 +645,7 @@ gfn_found:
     if(ret == 0) goto private_page_found;
         
     old_page = page;
-    page = mem_sharing_alloc_page(d, gfn);
+    page = alloc_domheap_page(d, 0); 
     if(!page) 
     {
         /* We've failed to obtain memory for private page. Need to re-add the
@@ -661,6 +653,7 @@ gfn_found:
         list_add(&gfn_info->list, &hash_entry->gfns);
         put_gfn(d, gfn);
         shr_unlock();
+        mem_sharing_notify_helper(d, gfn);
         return -ENOMEM;
     }
 
xen/arch/x86/mm/p2m.c +14 -20
diff -r 3c4c29899d8a -r 612f69531fd1 xen/arch/x86/mm/p2m.c
@@ -882,20 +882,12 @@ int p2m_mem_paging_evict(struct domain *
  */
 void p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn)
 {
-    struct vcpu *v = current;
-    mem_event_request_t req;
+    mem_event_request_t req = { .type = MEM_EVENT_TYPE_PAGING, .gfn = gfn };
 
-    /* Check that there's space on the ring for this request */
-    if ( mem_event_check_ring(d, &d->mem_event->paging) == 0)
-    {
-        /* Send release notification to pager */
-        memset(&req, 0, sizeof(req));
-        req.flags |= MEM_EVENT_FLAG_DROP_PAGE;
-        req.gfn = gfn;
-        req.vcpu_id = v->vcpu_id;
+    /* Send release notification to pager */
+    req.flags = MEM_EVENT_FLAG_DROP_PAGE;
 
-        mem_event_put_request(d, &d->mem_event->paging, &req);
-    }
+    mem_event_put_request(d, &d->mem_event->paging, &req);
 }
 
 /**
@@ -960,7 +952,7 @@ void p2m_mem_paging_populate(struct doma
     else if ( p2mt != p2m_ram_paging_out && p2mt != p2m_ram_paged )
     {
         /* gfn is already on its way back and vcpu is not paused */
-        mem_event_put_req_producers(&d->mem_event->paging);
+        mem_event_put_req_producers(d, &d->mem_event->paging);
         return;
     }
 
@@ -1074,8 +1066,8 @@ void p2m_mem_paging_resume(struct domain
     if ( rsp.flags & MEM_EVENT_FLAG_VCPU_PAUSED )
         vcpu_unpause(d->vcpu[rsp.vcpu_id]);
 
-    /* Unpause any domains that were paused because the ring was full */
-    mem_event_unpause_vcpus(d);
+    /* Wake vcpus waiting for room in the ring */
+    mem_event_wake_requesters(&d->mem_event->paging);
 }
 
 void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla, 
@@ -1114,7 +1106,7 @@ void p2m_mem_access_check(unsigned long 
                    "Memory access permissions failure, no mem_event listener: pausing VCPU %d, dom %d\n",
                    v->vcpu_id, d->domain_id);
 
-            mem_event_mark_and_pause(v);
+            mem_event_mark_and_sleep(v, &d->mem_event->access);
         }
         else
         {
@@ -1135,7 +1127,7 @@ void p2m_mem_access_check(unsigned long 
 
     /* Pause the current VCPU unconditionally */
     vcpu_pause_nosync(v);
-    req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;    
+    req.flags = MEM_EVENT_FLAG_VCPU_PAUSED;    
 
     /* Send request to mem event */
     req.gfn = gfn;
@@ -1164,9 +1156,11 @@ void p2m_mem_access_resume(struct p2m_do
     if ( rsp.flags & MEM_EVENT_FLAG_VCPU_PAUSED )
         vcpu_unpause(d->vcpu[rsp.vcpu_id]);
 
-    /* Unpause any domains that were paused because the ring was full or no listener 
-     * was available */
-    mem_event_unpause_vcpus(d);
+    /* Wake vcpus waiting for room in the ring */
+    mem_event_wake_requesters(&d->mem_event->access);
+
+    /* Unpause all vcpus that were paused because no listener was available */
+    mem_event_wake_waiters(d, &d->mem_event->access);
 }
 
 
xen/include/asm-x86/mem_event.h +4 -4
diff -r 3c4c29899d8a -r 612f69531fd1 xen/include/asm-x86/mem_event.h
@@ -24,13 +24,13 @@
 #ifndef __MEM_EVENT_H__
 #define __MEM_EVENT_H__
 
-/* Pauses VCPU while marking pause flag for mem event */
-void mem_event_mark_and_pause(struct vcpu *v);
 int mem_event_check_ring(struct domain *d, struct mem_event_domain *med);
-void mem_event_put_req_producers(struct mem_event_domain *med);
+void mem_event_put_req_producers(struct domain *d, struct mem_event_domain *med);
 void mem_event_put_request(struct domain *d, struct mem_event_domain *med, mem_event_request_t *req);
 void mem_event_get_response(struct mem_event_domain *med, mem_event_response_t *rsp);
-void mem_event_unpause_vcpus(struct domain *d);
+void mem_event_wake_requesters(struct mem_event_domain *med);
+void mem_event_wake_waiters(struct domain *d, struct mem_event_domain *med);
+void mem_event_mark_and_sleep(struct vcpu *v, struct mem_event_domain *med);
 
 int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
                      XEN_GUEST_HANDLE(void) u_domctl);
xen/include/xen/sched.h +12 -4
diff -r 3c4c29899d8a -r 612f69531fd1 xen/include/xen/sched.h
@@ -14,6 +14,7 @@
 #include <xen/nodemask.h>
 #include <xen/radix-tree.h>
 #include <xen/multicall.h>
+#include <xen/wait.h>
 #include <public/xen.h>
 #include <public/domctl.h>
 #include <public/sysctl.h>
@@ -183,7 +184,7 @@ struct mem_event_domain
 {
     /* ring lock */
     spinlock_t ring_lock;
-    unsigned int req_producers;
+    unsigned int foreign_producers;
     /* shared page */
     mem_event_shared_page_t *shared_page;
     /* shared ring page */
@@ -192,6 +193,10 @@ struct mem_event_domain
     mem_event_front_ring_t front_ring;
     /* event channel port (vcpu0 only) */
     int xen_port;
+    /* mem_event bit for vcpu->pause_flags */
+    int bit;
+    /* list of vcpus waiting for room in the ring */
+    struct waitqueue_head wq;
 };
 
 struct mem_event_per_domain
@@ -615,9 +620,12 @@ static inline struct domain *next_domain
  /* VCPU affinity has changed: migrating to a new CPU. */
 #define _VPF_migrating       3
 #define VPF_migrating        (1UL<<_VPF_migrating)
- /* VCPU is blocked on memory-event ring. */
-#define _VPF_mem_event       4
-#define VPF_mem_event        (1UL<<_VPF_mem_event)
+ /* VCPU is blocked due to missing mem_paging ring. */
+#define _VPF_mem_paging      4
+#define VPF_mem_paging       (1UL<<_VPF_mem_paging)
+ /* VCPU is blocked due to missing mem_access ring. */
+#define _VPF_mem_access      5
+#define VPF_mem_access       (1UL<<_VPF_mem_access)
 
 static inline int vcpu_runnable(struct vcpu *v)
 {
#2 lore raw

PATCH 1/1 xenpaging: use wait queues

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1322737507 -3600
# Node ID 8147822efdee65d1f5b94656ab2032aedb76979f # Parent 612f69531fd15cf59c58404f6e4762733a9a268c
xenpaging: use wait queues

Use a wait queue to put a guest vcpu to sleep while the requested gfn is in paging state. This adds missing p2m_mem_paging_populate() calls to some callers of the new get_gfn* variants, which would crash now because they get an invalid mfn. It also fixes guest crashes due to unexpected returns from do_memory_op because copy_to/from_guest ran into a paged gfn. Now those places will always get a valid mfn.

Since each gfn could be requested by several guest vcpus at the same time a queue of paged gfns is maintained. Each vcpu will be attached to that queue. Once p2m_mem_paging_resume restored the gfn the waiting vcpus will resume execution.

There is untested code in p2m_mem_paging_init_queue() to allow cpu hotplug. Since each vcpu may wait on a different gfn there have to be as many queues as vcpus. But xl vcpu-set does not seem to work right now, so this code path cant be excercised right now.

xen/arch/x86/hvm/hvm.c +2 -0
diff -r 612f69531fd1 -r 8147822efdee xen/arch/x86/hvm/hvm.c
@@ -454,6 +454,8 @@ int hvm_domain_initialise(struct domain 
     spin_lock_init(&d->arch.hvm_domain.irq_lock);
     spin_lock_init(&d->arch.hvm_domain.uc_lock);
 
+    spin_lock_init(&d->arch.hvm_domain.gfn_lock);
+
     INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);
     spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock);
 
xen/arch/x86/mm/p2m.c +203 -34

287 lines not on this page · raw message

xen/common/domctl.c +3 -0
diff -r 612f69531fd1 -r 8147822efdee xen/common/domctl.c
@@ -547,6 +547,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
                 goto maxvcpu_out;
         }
 
+        if ( p2m_mem_paging_init_queue(d, max) )
+            goto maxvcpu_out;
+
         ret = 0;
 
     maxvcpu_out:
xen/include/asm-x86/hvm/domain.h +3 -0
diff -r 612f69531fd1 -r 8147822efdee xen/include/asm-x86/hvm/domain.h
@@ -91,6 +91,9 @@ struct hvm_domain {
 
     struct viridian_domain viridian;
 
+    spinlock_t                        gfn_lock;
+    struct p2m_mem_paging_queue_head *gfn_queue;
+
     bool_t                 hap_enabled;
     bool_t                 mem_sharing_enabled;
     bool_t                 qemu_mapcache_invalidate;
xen/include/asm-x86/p2m.h +2 -0
diff -r 612f69531fd1 -r 8147822efdee xen/include/asm-x86/p2m.h
@@ -468,6 +468,8 @@ p2m_pod_offline_or_broken_replace(struct
 /* Modify p2m table for shared gfn */
 int set_shared_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
 
+/* Initialize per-gfn wait queue */
+int p2m_mem_paging_init_queue(struct domain *d, unsigned int max);
 /* Check if a nominated gfn is valid to be paged out */
 int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn);
 /* Evict a frame */
#5 lore raw

PATCH 1/1 xenpaging: add need_populate and paged_no_mfn checks

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1322737586 -3600
# Node ID c09ac3717a025a8ead44bbc795fedda715d134c7 # Parent 8147822efdee65d1f5b94656ab2032aedb76979f
xenpaging: add need_populate and paged_no_mfn checks

There is currently a mix of p2mt checks for the various paging types. Some mean the p2mt needs to be populated, others mean a gfn without mfn.

Add a new p2m_do_populate() helper which covers the p2m_ram_paged and p2m_ram_paging_out types. If a gfn is not in these states anymore another populate request for the pager is not needed. This avoids a call to p2m_mem_paging_populate() which in turn reduces the pressure on the ring buffer because no temporary slot needs to be claimed. As such, this helper is an optimization.

Modify the existing p2m_is_paged() helper which now covers also p2m_ram_paging_in_start in addition to the current p2m_ram_paged type. A gfn in these two states is not backed by a mfn.

xen/arch/x86/hvm/emulate.c +2 -1
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/hvm/emulate.c
@@ -66,7 +66,8 @@ static int hvmemul_do_io(
     ram_mfn = get_gfn_unshare(curr->domain, ram_gfn, &p2mt);
     if ( p2m_is_paging(p2mt) )
     {
-        p2m_mem_paging_populate(curr->domain, ram_gfn);
+        if ( p2m_do_populate(p2mt) )
+            p2m_mem_paging_populate(curr->domain, ram_gfn);
         put_gfn(curr->domain, ram_gfn); 
         return X86EMUL_RETRY;
     }
xen/arch/x86/hvm/hvm.c +11 -6
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/hvm/hvm.c
@@ -363,7 +363,8 @@ static int hvm_set_ioreq_page(
     }
     if ( p2m_is_paging(p2mt) )
     {
-        p2m_mem_paging_populate(d, gmfn);
+        if ( p2m_do_populate(p2mt) )
+            p2m_mem_paging_populate(d, gmfn);
         put_gfn(d, gmfn);
         return -ENOENT;
     }
@@ -1300,7 +1301,7 @@ int hvm_hap_nested_page_fault(unsigned l
 
 #ifdef __x86_64__
     /* Check if the page has been paged out */
-    if ( p2m_is_paged(p2mt) || (p2mt == p2m_ram_paging_out) )
+    if ( p2m_do_populate(p2mt) )
         p2m_mem_paging_populate(v->domain, gfn);
 
     /* Mem sharing: unshare the page and try again */
@@ -1820,7 +1821,8 @@ static void *__hvm_map_guest_frame(unsig
     }
     if ( p2m_is_paging(p2mt) )
     {
-        p2m_mem_paging_populate(d, gfn);
+        if ( p2m_do_populate(p2mt) )
+            p2m_mem_paging_populate(d, gfn);
         put_gfn(d, gfn);
         return NULL;
     }
@@ -2280,7 +2282,8 @@ static enum hvm_copy_result __hvm_copy(
 
         if ( p2m_is_paging(p2mt) )
         {
-            p2m_mem_paging_populate(curr->domain, gfn);
+            if ( p2m_do_populate(p2mt) )
+                p2m_mem_paging_populate(curr->domain, gfn);
             put_gfn(curr->domain, gfn);
             return HVMCOPY_gfn_paged_out;
         }
@@ -3760,7 +3763,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
             mfn_t mfn = get_gfn_unshare(d, pfn, &t);
             if ( p2m_is_paging(t) )
             {
-                p2m_mem_paging_populate(d, pfn);
+                if ( p2m_do_populate(t) )
+                    p2m_mem_paging_populate(d, pfn);
                 put_gfn(d, pfn);
                 rc = -EINVAL;
                 goto param_fail3;
@@ -3864,7 +3868,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
             mfn = get_gfn_unshare(d, pfn, &t);
             if ( p2m_is_paging(t) )
             {
-                p2m_mem_paging_populate(d, pfn);
+                if ( p2m_do_populate(t) )
+                    p2m_mem_paging_populate(d, pfn);
                 put_gfn(d, pfn);
                 rc = -EINVAL;
                 goto param_fail4;
xen/arch/x86/mm.c +15 -34

84 lines not on this page · raw message

xen/arch/x86/mm/guest_walk.c +2 -1
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/mm/guest_walk.c
@@ -102,7 +102,8 @@ static inline void *map_domain_gfn(struc
     if ( p2m_is_paging(*p2mt) )
     {
         ASSERT(!p2m_is_nestedp2m(p2m));
-        p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
+        if ( p2m_do_populate(*p2mt) )
+            p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
         __put_gfn(p2m, gfn_x(gfn));
         *rc = _PAGE_PAGED;
         return NULL;
xen/arch/x86/mm/hap/guest_walk.c +4 -2
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/mm/hap/guest_walk.c
@@ -64,7 +64,8 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
     if ( p2m_is_paging(p2mt) )
     {
         ASSERT(!p2m_is_nestedp2m(p2m));
-        p2m_mem_paging_populate(p2m->domain, cr3 >> PAGE_SHIFT);
+        if ( p2m_do_populate(p2mt) )
+            p2m_mem_paging_populate(p2m->domain, cr3 >> PAGE_SHIFT);
 
         pfec[0] = PFEC_page_paged;
         __put_gfn(p2m, top_gfn);
@@ -101,7 +102,8 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
         if ( p2m_is_paging(p2mt) )
         {
             ASSERT(!p2m_is_nestedp2m(p2m));
-            p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
+            if ( p2m_do_populate(p2mt) )
+                p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
 
             pfec[0] = PFEC_page_paged;
             __put_gfn(p2m, gfn_x(gfn));
xen/arch/x86/mm/p2m-ept.c +1 -2
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/mm/p2m-ept.c
@@ -375,8 +375,7 @@ ept_set_entry(struct p2m_domain *p2m, un
          * Read-then-write is OK because we hold the p2m lock. */
         old_entry = *ept_entry;
 
-        if ( mfn_valid(mfn_x(mfn)) || direct_mmio || p2m_is_paged(p2mt) ||
-             (p2mt == p2m_ram_paging_in_start) )
+        if ( mfn_valid(mfn_x(mfn)) || direct_mmio || p2m_is_paged(p2mt) )
         {
             /* Construct the new entry, and then write it once */
             new_entry.emt = epte_get_entry_emt(p2m->domain, gfn, mfn, &ipat,
xen/arch/x86/mm/p2m.c +2 -2
diff -r 8147822efdee -r c09ac3717a02 xen/arch/x86/mm/p2m.c
@@ -313,7 +313,7 @@ static void p2m_mem_paging_wait(mfn_t *m
         return;
 
     /* Populate the page once */
-    if ( *t == p2m_ram_paging_out || *t == p2m_ram_paged )
+    if ( p2m_do_populate(*t) )
         p2m_mem_paging_populate(p2m->domain, gfn);
 
     wait_event(pmpq->wq, p2m_mem_paging_get_entry(mfn, p2m, gfn, t, a, q, page_order));
@@ -1111,7 +1111,7 @@ void p2m_mem_paging_populate(struct doma
     p2m_lock(p2m);
     mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
     /* Forward the state only if gfn is in page-out path */
-    if ( p2mt == p2m_ram_paging_out || p2mt == p2m_ram_paged ) {
+    if ( p2m_do_populate(p2mt) ) {
         /* Ignore foreign requests to allow mmap in pager */
         if ( mfn_valid(mfn) && p2mt == p2m_ram_paging_out && v->domain == d ) {
             /* Restore gfn because it is needed by guest before evict */
xen/common/grant_table.c +2 -1
diff -r 8147822efdee -r c09ac3717a02 xen/common/grant_table.c
@@ -163,7 +163,8 @@ static int __get_paged_frame(unsigned lo
         *frame = mfn_x(mfn);
         if ( p2m_is_paging(p2mt) )
         {
-            p2m_mem_paging_populate(rd, gfn);
+            if ( p2m_do_populate(p2mt) )
+                p2m_mem_paging_populate(rd, gfn);
             put_gfn(rd, gfn);
             rc = GNTST_eagain;
         }
xen/include/asm-x86/p2m.h +7 -2
diff -r 8147822efdee -r c09ac3717a02 xen/include/asm-x86/p2m.h
@@ -158,7 +158,11 @@ typedef enum {
                           | p2m_to_mask(p2m_ram_paging_in_start) \
                           | p2m_to_mask(p2m_ram_paging_in))
 
-#define P2M_PAGED_TYPES (p2m_to_mask(p2m_ram_paged))
+#define P2M_POPULATE_TYPES (p2m_to_mask(p2m_ram_paged) \
+                            | p2m_to_mask(p2m_ram_paging_out) )
+
+#define P2M_PAGED_NO_MFN_TYPES (p2m_to_mask(p2m_ram_paged) \
+                               | p2m_to_mask(p2m_ram_paging_in_start) )
 
 /* Shared types */
 /* XXX: Sharable types could include p2m_ram_ro too, but we would need to
@@ -183,7 +187,8 @@ typedef enum {
 #define p2m_has_emt(_t)  (p2m_to_mask(_t) & (P2M_RAM_TYPES | p2m_to_mask(p2m_mmio_direct)))
 #define p2m_is_pageable(_t) (p2m_to_mask(_t) & P2M_PAGEABLE_TYPES)
 #define p2m_is_paging(_t)   (p2m_to_mask(_t) & P2M_PAGING_TYPES)
-#define p2m_is_paged(_t)    (p2m_to_mask(_t) & P2M_PAGED_TYPES)
+#define p2m_do_populate(_t) (p2m_to_mask(_t) & P2M_POPULATE_TYPES)
+#define p2m_is_paged(_t)    (p2m_to_mask(_t) & P2M_PAGED_NO_MFN_TYPES)
 #define p2m_is_sharable(_t) (p2m_to_mask(_t) & P2M_SHARABLE_TYPES)
 #define p2m_is_shared(_t)   (p2m_to_mask(_t) & P2M_SHARED_TYPES)
 #define p2m_is_broken(_t)   (p2m_to_mask(_t) & P2M_BROKEN_TYPES)
#6 lore raw

PATCH 1/1 Add tracing to mem_event and p2m_mem_paging functions

In reply to Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1322737677 -3600
# Node ID 0bf827a48a3c9efd43fe04c291b553d2bec46f8b # Parent c09ac3717a025a8ead44bbc795fedda715d134c7 Add tracing to mem_event and p2m_mem_paging functions

Maintaining these trace_var calls out-of-tree became a burden and made debugging paging related issues harder. So putting it into the tree will ease debugging with xenalyze.

xen/arch/x86/mm/mem_event.c +18 -2
diff -r c09ac3717a02 -r 0bf827a48a3c xen/arch/x86/mm/mem_event.c
@@ -24,6 +24,7 @@
 #include <asm/domain.h>
 #include <xen/event.h>
 #include <xen/wait.h>
+#include <xen/trace.h>
 #include <asm/p2m.h>
 #include <asm/mem_event.h>
 #include <asm/mem_paging.h>
@@ -149,6 +150,13 @@ static int _mem_event_put_request(struct
     mem_event_front_ring_t *front_ring;
     int free_requests;
     RING_IDX req_prod;
+    struct trc_mem_event_put_request T = { .td = d->domain_id };
+    int ret = 0;
+
+    T.d = current->domain->domain_id;
+    T.v = current->vcpu_id;
+    T.ring_bit = med->bit;
+    T.gfn = req->gfn;
 
     mem_event_ring_lock(med);
 
@@ -156,7 +164,7 @@ static int _mem_event_put_request(struct
     /* Requests from foreign domain were claimed in mem_event_check_ring() */
     if ((current->domain == d && free_requests < med->foreign_producers) || !free_requests) {
         mem_event_ring_unlock(med);
-        return 0;
+        goto out;
     }
 
     front_ring = &med->front_ring;
@@ -176,7 +184,15 @@ static int _mem_event_put_request(struct
 
     notify_via_xen_event_channel(d, med->xen_port);
 
-    return 1;
+    ret = 1;
+
+out:
+    T.ret = ret;
+    T.room = free_requests;
+    T.foreign_producers = med->foreign_producers + ret;
+    trace_var(TRC_MEM_EVENT_PUT_REQUEST, 1, sizeof(T), &T);
+
+    return ret;
 }
 
 void mem_event_put_request(struct domain *d, struct mem_event_domain *med,
xen/arch/x86/mm/p2m.c +115 -8

295 lines not on this page · raw message

xen/include/public/trace.h +31 -0
diff -r c09ac3717a02 -r 0bf827a48a3c xen/include/public/trace.h
@@ -94,6 +94,37 @@
 #define TRC_MEM_POD_ZERO_RECLAIM    (TRC_MEM + 17)
 #define TRC_MEM_POD_SUPERPAGE_SPLINTER (TRC_MEM + 18)
 
+#define TRC_MEM_EVENT_PUT_REQUEST      (TRC_MEM + 19)
+struct trc_mem_event_put_request {
+    unsigned int d:16, v:16;
+    unsigned int td:16, room:16;
+    unsigned int gfn;
+    unsigned int foreign_producers:16, ret:1, ring_bit:5;
+} __attribute__((packed));
+
+#define TRC_MEM_P2M_PAGING_NOMINATE    (TRC_MEM + 20)
+#define TRC_MEM_P2M_PAGING_EVICT       (TRC_MEM + 21)
+#define TRC_MEM_P2M_PAGING_DROP        (TRC_MEM + 22)
+#define TRC_MEM_P2M_PAGING_POPULATE    (TRC_MEM + 23)
+#define TRC_MEM_P2M_PAGING_PREP        (TRC_MEM + 24)
+#define TRC_MEM_P2M_PAGING_RESUME      (TRC_MEM + 25)
+struct trc_mem_p2m_paging {
+    unsigned int d:16, v:16;
+    unsigned int td:16, p2mt:5, reason:5, flag_evict_fail:1, flag_drop_page:1;
+    unsigned int gfn;
+    unsigned int mfn;
+} __attribute__((packed));
+
+#define TRC_MEM_P2M_PAGING_GET_QUEUE   (TRC_MEM + 26)
+#define TRC_MEM_P2M_PAGING_PUT_QUEUE   (TRC_MEM + 27)
+#define TRC_MEM_P2M_PAGING_WAKE_QUEUE  (TRC_MEM + 28)
+struct trc_mem_p2m_paging_queue {
+    unsigned int d:16, v:16;
+    unsigned int td:16, woken:8;
+    unsigned int gfn;
+    unsigned int index:16, waiters:16;
+} __attribute__((packed));
+
 
 #define TRC_PV_HYPERCALL             (TRC_PV +  1)
 #define TRC_PV_TRAP                  (TRC_PV +  3)
#7 lore raw
In reply to Olaf Hering
Quoted 5 lines · Olaf Hering

The following series is my current state of an attempt to use wait queues for xenpaging and mem_event handling. I post it here for review and comments.

This series conflicts with work from Andres Lagar-Cavilla.

Can you be clear about which of Andres's patchsets you are objecting to? I have five series (!) of his on my todo list right now.

Tim.

#8 lore raw
Quoted 9 lines · Tim Deegan
Quoted 5 lines · Olaf Hering

The following series is my current state of an attempt to use wait queues for xenpaging and mem_event handling. I post it here for review and comments.

This series conflicts with work from Andres Lagar-Cavilla.

Can you be clear about which of Andres's patchsets you are objecting to? I have five series (!) of his on my todo list right now.

The "Improve ring management for memory events" change which tries to achieve the same result as my "mem_event: use wait queue when ring is full" change.
There is already a discussion about which way to choose.

Olaf

#3 lore raw

Re: [PATCH 2 of 4] xenpaging: use wait queues

In reply to Olaf Hering

Hi,

Is there a more recent version of this patch? I see a newer version of the wait-queues-for-mem-events one but not of this.

Quoted 31 lines · Olaf Hering

# HG changeset patch
# User Olaf Hering <[email protected]>
# Date 1322737507 -3600
# Node ID 8147822efdee65d1f5b94656ab2032aedb76979f # Parent 612f69531fd15cf59c58404f6e4762733a9a268c
xenpaging: use wait queues

Use a wait queue to put a guest vcpu to sleep while the requested gfn is in paging state. This adds missing p2m_mem_paging_populate() calls to some callers of the new get_gfn* variants, which would crash now because they get an invalid mfn. It also fixes guest crashes due to unexpected returns from do_memory_op because copy_to/from_guest ran into a paged gfn. Now those places will always get a valid mfn.

Since each gfn could be requested by several guest vcpus at the same time a queue of paged gfns is maintained. Each vcpu will be attached to that queue. Once p2m_mem_paging_resume restored the gfn the waiting vcpus will resume execution.

There is untested code in p2m_mem_paging_init_queue() to allow cpu hotplug. Since each vcpu may wait on a different gfn there have to be as many queues as vcpus. But xl vcpu-set does not seem to work right now, so this code path cant be excercised right now.

diff -r 612f69531fd1 -r 8147822efdee xen/arch/x86/hvm/hvm.c

xen/arch/x86/hvm/hvm.c int hvm_domain_initialise(struct domain · line 458

Quoted 5 lines
xen/arch/x86/hvm/hvm.c +2 -0
@@ -454,6 +454,8 @@ int hvm_domain_initialise(struct domain 
     spin_lock_init(&d->arch.hvm_domain.irq_lock);
     spin_lock_init(&d->arch.hvm_domain.uc_lock);
 
+    spin_lock_init(&d->arch.hvm_domain.gfn_lock);
+

I agree with Andres's comments on this lock, and will go futher: please
- add it to the mm-locks.h system so we see any ordering vioations in future patches.
- add a detailed comment there explaining what exactly this lock protects.

Cheers,

Tim.

#4 lore raw

Re: [PATCH 2 of 4] xenpaging: use wait queues

Quoted 2 lines · Tim Deegan

Is there a more recent version of this patch? I see a newer version of the wait-queues-for-mem-events one but not of this.

The version I sent is not final, pv guests wont start for example. I will adress the comments from Andres and you and send out a better version.

Olaf