Re: alloc_heap_pages is low efficient with more CPUs

[Keir Fraser <keir@xen.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2861 bytes --]

If the allocations are 2M size, we can do better quite easily I think.
Please try the attached patch.

 -- Keir

On 13/10/2012 07:46, "tupeng212" <tupeng212@gmail.com> wrote:

> What if you replace tlbflush_filter() call with cpus_clear(&extra_cpus_mask)?
> :  you mean just clear it,  maybe a little violent..,  you 'd like to do it at
> any other place.
>  
> I assume you see lots of looping in one of those two functions, but only
> single-page-at-a-time calls into alloc_domheap_pages()->alloc_heap_pages()?
> :  In populate_physmap,  all pages are 2M size,
> static void populate_physmap(struct memop_args *a)
> {
>     for ( i = a->nr_done; i < a->nr_extents; i++ )
>     {
> page = alloc_domheap_pages(d, a->extent_order, a->memflags) ->alloc_heap_pages
> ;  //a->extent_order = 9, always 2M size
> }
> //you mean move that block and TLB-flush here to avoid for loop ?
> }
> 
> tupeng212
>  
> From: Keir Fraser <mailto:keir.xen@gmail.com>
> Date: 2012-10-13 14:30
> To: tupeng212 <mailto:tupeng212@gmail.com>
> Subject: Re: [Xen-devel] alloc_heap_pages is low efficient with more CPUs
> What if you replace tlbflush_filter() call with cpus_clear(&extra_cpus_mask)?
> Seems a bit silly to do, but I¹d like to know how much a few cpumask
> operations per page is costing (most are of course much quicker than
> tlbflush_filter as they operate on 64 bits per iteration, rather than one bit
> per iteration).
> 
> If that is suitably fast, I think we can have a go at fixing this by pulling
> the TLB-flush logic out of alloc_heap_pages() and into the loops in
> increwase_reservation() and populate_physmap() in common/memory.c. I assume
> you see lots of looping in one of those two functions, but only
> single-page-at-a-time calls into alloc_domheap_pages()->alloc_heap_pages()?
> 
>   -- Keir
> 
> On 13/10/2012 07:21, "tupeng212" <tupeng212@gmail.com> wrote:
> 
>> If the  tlbflush_filter() and cpumask_or() lines are commented out from  the
>> if(need_tlbflush) block in alloc_heap_pages(), what are the domain  creation
>> times like then?
>> : You mean removing these code from  alloc_heap_pages, then try it.
>> I didn't do it as you said, but I calculated  the whole time of
>> if(need_tlbflush) block
>> by using time1=NOW() ...block ...  time2=NOW(), time=time2-time1, its unit is
>> ns, and s = ns * 10^9
>> it occupy  high rate of the whole time. whole starting time is 30s, then this
>> block may  be 29s.
>>  
>> By the way it looks like you are not using xen-unstable  or
>> xen-4.2, can you try with one of these later versions of Xen?
>> :  fortunately, other groups have to use xen-4.2, we have repeated this
>> experiment on 
>> that source code too, it changed nothing, time is still very  long in second
>> starting.
>>  
>>  
>> 
>>  tupeng
>>  
>>  
>> 
> 


[-- Attachment #1.2: Type: text/html, Size: 4367 bytes --]

[-- Attachment #2: 00-reduce-tlbflush_filter --]
[-- Type: application/octet-stream, Size: 2242 bytes --]

diff -r a15596a619ed xen/common/page_alloc.c
--- a/xen/common/page_alloc.c	Thu Oct 04 10:44:43 2012 +0200
+++ b/xen/common/page_alloc.c	Sat Oct 13 09:57:26 2012 +0100
@@ -303,9 +303,10 @@ static struct page_info *alloc_heap_page
     unsigned int first_node, i, j, zone = 0, nodemask_retry = 0;
     unsigned int node = (uint8_t)((memflags >> _MEMF_node) - 1);
     unsigned long request = 1UL << order;
-    cpumask_t extra_cpus_mask, mask;
     struct page_info *pg;
     nodemask_t nodemask = (d != NULL ) ? d->node_affinity : node_online_map;
+    bool_t need_tlbflush = 0;
+    uint32_t tlbflush_timestamp = 0;
 
     if ( node == NUMA_NO_NODE )
     {
@@ -417,20 +418,19 @@ static struct page_info *alloc_heap_page
     if ( d != NULL )
         d->last_alloc_node = node;
 
-    cpus_clear(mask);
-
     for ( i = 0; i < (1 << order); i++ )
     {
         /* Reference count must continuously be zero for free pages. */
         BUG_ON(pg[i].count_info != PGC_state_free);
         pg[i].count_info = PGC_state_inuse;
 
-        if ( pg[i].u.free.need_tlbflush )
+        if ( pg[i].u.free.need_tlbflush &&
+             (pg[i].tlbflush_timestamp <= tlbflush_current_time()) &&
+             (!need_tlbflush ||
+              (pg[i].tlbflush_timestamp > tlbflush_timestamp)) )
         {
-            /* Add in extra CPUs that need flushing because of this page. */
-            cpus_andnot(extra_cpus_mask, cpu_online_map, mask);
-            tlbflush_filter(extra_cpus_mask, pg[i].tlbflush_timestamp);
-            cpus_or(mask, mask, extra_cpus_mask);
+            need_tlbflush = 1;
+            tlbflush_timestamp = pg[i].tlbflush_timestamp;
         }
 
         /* Initialise fields which have other uses for free pages. */
@@ -440,10 +440,15 @@ static struct page_info *alloc_heap_page
 
     spin_unlock(&heap_lock);
 
-    if ( unlikely(!cpus_empty(mask)) )
+    if ( need_tlbflush )
     {
-        perfc_incr(need_flush_tlb_flush);
-        flush_tlb_mask(&mask);
+        cpumask_t mask = cpu_online_map;
+        tlbflush_filter(mask, tlbflush_timestamp);
+        if ( !cpus_empty(mask) )
+        {
+            perfc_incr(need_flush_tlb_flush);
+            flush_tlb_mask(&mask);
+        }
     }
 
     return pg;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel