Rework populate-on-demand sweeping

Quoted 1 line

Quoted 1 line · George Dunlap

When demand-populating pages due to guest accesses, check recently populated pages to see if we can reclaim them for the cache. This should keep the PoD cache filled when the start-of-day scrubber is going through.

The number 128 was chosen by experiment. Windows does its page scrubbing in parallel; while a small nubmer like 4 works well for single VMs, it breaks down as multiple vcpus are scrubbing different pages in parallel. Increasing to 128 works well for higher numbers of vcpus.

Signed-off-by George Dunlap <[email protected]>

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

@@ -909,6 +909,26 @@ p2m_pod_emergency_sweep_super(struct p2m
     p2m->pod.reclaim_super = i ? i - SUPERPAGE_PAGES : 0;
 }
 
+/* When populating a new superpage, look at recently populated superpages
+ * hoping that they've been zeroed.  This will snap up zeroed pages as soon as 
+ * the guest OS is done with them. */
+static void
+p2m_pod_check_last_super(struct p2m_domain *p2m, unsigned long gfn_aligned)
+{
+    unsigned long check_gfn;
+
+    ASSERT(p2m->pod.last_populated_index < POD_HISTORY_MAX);
+
+    check_gfn = p2m->pod.last_populated[p2m->pod.last_populated_index];
+
+    p2m->pod.last_populated[p2m->pod.last_populated_index] = gfn_aligned;
+
+    p2m->pod.last_populated_index = ( p2m->pod.last_populated_index + 1 ) % POD_HISTORY_MAX;
+
+    p2m->pod.reclaim_super += p2m_pod_zero_check_superpage(p2m, check_gfn);
+}
+
+
 #define POD_SWEEP_STRIDE  16
 static void
 p2m_pod_emergency_sweep(struct p2m_domain *p2m)

@@ -1066,6 +1086,12 @@ p2m_pod_demand_populate(struct p2m_domai
         __trace_var(TRC_MEM_POD_POPULATE, 0, sizeof(t), &t);
     }
 
+    /* Check the last guest demand-populate */
+    if ( p2m->pod.entry_count > p2m->pod.count 
+         && order == 9
+         && q & P2M_ALLOC )
+        p2m_pod_check_last_super(p2m, gfn_aligned);
+
     pod_unlock(p2m);
     return 0;
 out_of_memory:

xen/include/asm-x86/p2m.h +0 -0

         mm_lock_t        lock;         /* Locking of private pod structs,   
 *
                                         * not relying on the p2m lock.      

*/

     } pod;

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

xen/include/asm-x86/p2m.h struct p2m_domain { · line 291

Quoted 9 lines

Quoted 1 line

Quoted 1 line · George Dunlap

xen/include/asm-x86/p2m.h +2 -0

@@ -287,6 +287,9 @@ struct p2m_domain {
         unsigned         reclaim_super; /* Last gpfn of a scan */
         unsigned         reclaim_single; /* Last gpfn of a scan */
         unsigned         max_guest;    /* gpfn of max guest demand-populate */
+#define POD_HISTORY_MAX 128
+        unsigned         last_populated[POD_HISTORY_MAX]; /* gpfn of last guest page demand-populated */

unsigned long?

Also, wouldn't it be better to allocate this table dynamically, at once allowing its size to scale with the number of vCPU-s in the guest?

I haven't done more than compile-test it at this point, so please just review ideas and "is this 4.2 material". If I get positive feedback, I'll do more testing and re-submit.

Quoted 7 lines · George Dunlap

+    if ( p2m->pod.count == 0 )
+        goto out_of_memory;

     /* Keep track of the highest gfn demand-populated by a guest fault */ if ( gfn > p2m->pod.max_guest )
         p2m->pod.max_guest = gfn;

Quoted 3 lines

-    if ( p2m->pod.count == 0 )
-        goto out_of_memory;
-

Is this code motion just noise?  Since out_of_memory crasheds the guest, I assume so.

Quoted 12 lines · Tim Deegan

Quoted 10 lines · George Dunlap

+    if ( p2m->pod.count == 0 )
+        goto out_of_memory;

     /* Keep track of the highest gfn demand-populated by a guest fault */ if ( gfn > p2m->pod.max_guest )
         p2m->pod.max_guest = gfn;

-    if ( p2m->pod.count == 0 )
-        goto out_of_memory;
-

Quoted 2 lines

Is this code motion just noise?  Since out_of_memory crasheds the guest, I assume so.

It will have no practical effect, other than improving the efficiency of crashing a guest by a few cycles, if that's what you're asking. It's more about taste: it just seemed silly to make an assignment and *then* see if you were going to crash. :-)

Quoted 16 lines · George Dunlap

Quoted 14 lines · Tim Deegan

Quoted 10 lines · George Dunlap

+    if ( p2m->pod.count == 0 )
+        goto out_of_memory;

     /* Keep track of the highest gfn demand-populated by a guest fault */ if ( gfn > p2m->pod.max_guest )
         p2m->pod.max_guest = gfn;

-    if ( p2m->pod.count == 0 )
-        goto out_of_memory;
-

Is this code motion just noise?  Since out_of_memory crasheds the guest, I assume so.

Quoted 4 lines

It will have no practical effect, other than improving the efficiency of crashing a guest by a few cycles, if that's what you're asking. It's more about taste: it just seemed silly to make an assignment and *then* see if you were going to crash. :-)

Well, yes. :)  I don't think it quite falls under the description of the patch, though.  Can you split it out as a cleanup patch?

Quoted 1 line · Jan Beulich

Quoted 1 line

Quoted 1 line

Quoted 1 line · George Dunlap

Quoted 8 lines

xen/include/asm-x86/p2m.h struct p2m_domain { · line 291

Quoted 8 lines

xen/include/asm-x86/p2m.h +2 -0

@@ -287,6 +287,9 @@ struct p2m_domain {
          unsigned         reclaim_super; /* Last gpfn of a scan */
          unsigned         reclaim_single; /* Last gpfn of a scan */
          unsigned         max_guest;    /* gpfn of max guest demand-populate */
+#define POD_HISTORY_MAX 128
+        unsigned         last_populated[POD_HISTORY_MAX]; /* gpfn of last guest page demand-populated */

This is the gpfns of the last 128 order-9 superpages populated, right?

Also, this line is>80 columns - I think I saw a few others in this series.

Quoted 6 lines

unsigned long?

Also, wouldn't it be better to allocate this table dynamically, at once allowing its size to scale with the number of vCPU-s in the guest?

You could even make it a small per-vcpu array, assuming that the parallel scrubbing will be symmetric across vcpus.

Quoted 5 lines

Quoted 3 lines

Also, wouldn't it be better to allocate this table dynamically, at once allowing its size to scale with the number of vCPU-s in the guest?

You could even make it a small per-vcpu array, assuming that the parallel scrubbing will be symmetric across vcpus.

I can't remember exactly what I found here (this was last summer I was doing the tests); it may be that Windows creates a bunch of tasks which may migrate to various cpus. If that were the case, a global list would be better than per-vcpu lists.

The problem with dynamically scaling the list is that I don't have a heuristic to hand for how to scale it.

In both cases, it's not unlikely that making a change without testing will significantly reduce the effectiveness of the patch. Would you rather hold off and wait until I can get a chance to run my benchmarks again (which may miss the 4.2 cycle), or accept a tidied-up version of this patch first, and hope to get a revised method (using dynamic scaling or per-vcpu arrays) in before 4.2, but for sure by 4.3?