WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] [PATCH] When flush tlb , we need consider the cpu_online

To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] When flush tlb , we need consider the cpu_online_map
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Tue, 30 Mar 2010 11:29:29 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 29 Mar 2010 20:31:55 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C7D678B7.EBBD%keir.fraser@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4BB0BF4D0200007800037763@xxxxxxxxxxxxxxxxxx> <C7D678B7.EBBD%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcrPPu8ogIZutj03T5OTWemC01hHvwADdKsgABrfloA=
Thread-topic: [Xen-devel] [PATCH] When flush tlb , we need consider the cpu_online_map
Sure.Attached is the new patch. 

BTW, Xiantao will have a look on the two cpu_clear(cpu, cpu_online_map) in 
__cpu_disable(), so I remove the comments in the patch.

Thanks
--jyh


When flush tlb mask, we need consider the cpu_online_map. The same happens to 
ept flush also.

We noticed sometime system hang on cpu online/offline stress test. The reason 
is because flush_tlb_mask from __get_page_type is deadloop.

This should be caused by a small windows in cpu offline.
The cpu_online_map is changed and the interrupt is disabled at take_cpu_down() 
for the to-be-offline CPU.

However, the __sync_lazy_execstate() called from idle_task_exit() in the 
idle_loop() for the to-be-offline CPU. At that time, the stop_machine_run is 
finished already, and __get_page_type may be called in other CPU before the 
__sync_lazy_execstate().

Thanks Jan pointing out issue in my original patch and gives suggestion, not 
sure if he would like be in the signed-off-by.

Signed-off-by: Jiang, Yunhong <yunhong.jiang@xxxxxxxxx>

diff -r f3db0ae08304 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c        Sat Mar 27 16:01:35 2010 +0000
+++ b/xen/arch/x86/hvm/vmx/vmx.c        Tue Mar 30 11:07:33 2010 +0800
@@ -1234,7 +1234,9 @@ void ept_sync_domain(struct domain *d)
      * the ept_synced mask before on_selected_cpus() reads it, resulting in
      * unnecessary extra flushes, to avoid allocating a cpumask_t on the stack.
      */
-    d->arch.hvm_domain.vmx.ept_synced = d->domain_dirty_cpumask;
+    cpus_and(d->arch.hvm_domain.vmx.ept_synced,
+                 d->domain_dirty_cpumask, cpu_online_map);
+
     on_selected_cpus(&d->arch.hvm_domain.vmx.ept_synced,
                      __ept_sync_domain, d, 1);
 }
diff -r f3db0ae08304 xen/arch/x86/smp.c
--- a/xen/arch/x86/smp.c        Sat Mar 27 16:01:35 2010 +0000
+++ b/xen/arch/x86/smp.c        Tue Mar 30 11:06:59 2010 +0800
@@ -228,7 +228,8 @@ void flush_area_mask(const cpumask_t *ma
     if ( !cpus_subset(*mask, *cpumask_of(smp_processor_id())) )
     {
         spin_lock(&flush_lock);
-        cpus_andnot(flush_cpumask, *mask, *cpumask_of(smp_processor_id()));
+        cpus_and(flush_cpumask, *mask, cpu_online_map);
+        cpu_clear(smp_processor_id(), flush_cpumask);
         flush_va      = va;
         flush_flags   = flags;
         send_IPI_mask(&flush_cpumask, INVALIDATE_TLB_VECTOR);



>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
>Sent: Monday, March 29, 2010 10:33 PM
>To: Jan Beulich; Jiang, Yunhong
>Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-devel] [PATCH] When flush tlb , we need consider the
>cpu_online_map
>
>Sounds good. Can you please re-spin the patch, Yunhong? I will drop your
>original patch for now.
>
> -- Keir
>
>On 29/03/2010 13:55, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
>
>>>>> "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx> 29.03.10 14:00 >>>
>>> When flush tlb mask, we need consider the cpu_online_map. The same happens
>to
>>> ept flush also.
>>
>> While the idea is certainly correct, doing this more efficiently seems
>> quite desirable to me, especially when NR_CPUS is large:
>>
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c Sat Mar 27 16:01:35 2010 +0000
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c Mon Mar 29 17:49:51 2010 +0800
>>> @@ -1235,6 +1235,9 @@ void ept_sync_domain(struct domain *d)
>>>      * unnecessary extra flushes, to avoid allocating a cpumask_t on the
>>> stack.
>>>      */
>>>     d->arch.hvm_domain.vmx.ept_synced = d->domain_dirty_cpumask;
>>> +    cpus_and(d->arch.hvm_domain.vmx.ept_synced,
>>> +             d->arch.hvm_domain.vmx.ept_synced,
>>> +             cpu_online_map);
>>
>> The added code can be combined with the pre-existing line:
>>
>>     cpus_and(d->arch.hvm_domain.vmx.ept_synced,
>>              d->domain_dirty_cpumask, cpu_online_map);
>>
>>>     on_selected_cpus(&d->arch.hvm_domain.vmx.ept_synced,
>>>                      __ept_sync_domain, d, 1);
>>> }
>>> --- a/xen/arch/x86/smp.c Sat Mar 27 16:01:35 2010 +0000
>>> +++ b/xen/arch/x86/smp.c Mon Mar 29 17:47:25 2010 +0800
>>> @@ -229,6 +229,7 @@ void flush_area_mask(const cpumask_t *ma
>>>     {
>>>         spin_lock(&flush_lock);
>>>         cpus_andnot(flush_cpumask, *mask,
>*cpumask_of(smp_processor_id()));
>>> +        cpus_and(flush_cpumask, cpu_online_map, flush_cpumask);
>>
>> Here, first doing the full-mask operation and then clearing the one
>> extra bit is less overhead:
>>
>>         cpus_and(flush_cpumask, *mask, cpu_online_map);
>>         cpu_clear(smp_processor_id(), flush_cpumask);
>>
>>>         flush_va      = va;
>>>         flush_flags   = flags;
>>>         send_IPI_mask(&flush_cpumask, INVALIDATE_TLB_VECTOR);
>>
>> Jan
>>
>>
>>
>

Attachment: flush_tlb_map.patch
Description: flush_tlb_map.patch

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