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 3/3] Disable ARB_DIS conditionally

To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH 3/3] Disable ARB_DIS conditionally
From: "Wei, Gang" <gang.wei@xxxxxxxxx>
Date: Wed, 3 Sep 2008 18:21:50 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Wed, 03 Sep 2008 03:22:22 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C4E40C1B.1CC6D%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: <8FED46E8A9CA574792FC7AACAC38FE770180652418@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <C4E40C1B.1CC6D%keir.fraser@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AckNaBRbmlf20lzgRmapdIQh+qyTZAAOKwDGAAFiUcA=
Thread-topic: [Xen-devel] [PATCH 3/3] Disable ARB_DIS conditionally
Thanks for point it out. Of couse we should not let ARB_DISABLE set to 1 while 
some CPUs are not idling.

The real racing case I can imagine is as below:

CPU0: -------------judge then clear 
ARB_DIS----------------------------------dec count
CPU1: inc count-------------------------------------judge then set ARB_DIS

Sync via spinlock is good suggest, I need to do some testing to make sure 
spinlock will not bring larger-than-saved overheads.

Please hold off this patch.

Jimmy

On Wednesday, September 03, 2008 4:41 PM, Keir Fraser wrote:
> I'm not sure about how safe it would be to have ARB_DISABLE set to 1 while
> CPUs are not idling. This patch has race that could allow that to happen:
>
> Two CPUs stop idling, one sets ARB_DISABLE=0 but before it decrements
> c3_cpu_count the other does so, then re-enters the idle function,
> re-increments c3_cpu_count, and sets ARB_DISABLE=1. We now have at least one
> CPU running normally yet ARB_DISABLE=1.
>
> If that race is bad then either we have to leave as is, or synchronise on a
> spinlock.
>
>  -- Keir
>
> On 3/9/08 02:55, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
>
>> CPUIDLE: Write to ARB_DISABLE conditionally to reduce some idle overheads.
>>
>> Signed-off-by: Wei Gang <gang.wei@xxxxxxxxx>
>>
>> diff -r 48ab8d09f41e xen/arch/x86/acpi/cpu_idle.c
>> --- a/xen/arch/x86/acpi/cpu_idle.c      Tue Sep 02 15:38:37 2008 +0800
>> +++ b/xen/arch/x86/acpi/cpu_idle.c      Tue Sep 02 16:03:09 2008 +0800
>> @@ -455,8 +455,9 @@ static void acpi_processor_idle(void)
>>          if ( power->flags.bm_check && power->flags.bm_control )          {
>>              /* Enable bus master arbitration */
>> +            if ( atomic_read(&c3_cpu_count) == num_online_cpus() )
>> +                acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
>>              atomic_dec(&c3_cpu_count);
>> -            acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);          }
>>
>>          /* Re-enable interrupts */
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel

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