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

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

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