RE: [Xen-devel] [PATCH 3/3] Disable ARB_DIS conditionally

["Wei, Gang" <gang.wei@xxxxxxxxx>]

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