x86/idle: reduce contention on ACPI register accesses Other than when they're located in I/O port space, accessing them when in MMIO space (currently) implies usage of some sort of global lock: In -unstable this would be due to the use of vmap(), is older trees the necessary locking was introduced by 2ee9cbf9 ("ACPI: fix acpi_os_map_memory()"). This contention was observed to result in Dom0 kernel soft lockups during the loading of the ACPI processor driver there on systems with very many CPU cores. There are a couple of things being done for this: - re-order elements of an if() condition so that the register access only happens when we really need it - turn off arbitration disabling only when the first CPU leaves C3 (paralleling how arbitration disabling gets turned on) - only set the (global) bus master reload flag once (when the first target CPU gets processed) Signed-off-by: Jan Beulich --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -439,8 +439,8 @@ static void acpi_processor_idle(void) (next_state = cpuidle_current_governor->select(power)) > 0 ) { cx = &power->states[next_state]; - if ( power->flags.bm_check && acpi_idle_bm_check() - && cx->type == ACPI_STATE_C3 ) + if ( cx->type == ACPI_STATE_C3 && power->flags.bm_check && + acpi_idle_bm_check() ) cx = power->safe_state; if ( cx->idx > max_cstate ) cx = &power->states[max_cstate]; @@ -563,8 +563,8 @@ static void acpi_processor_idle(void) { /* Enable bus master arbitration */ spin_lock(&c3_cpu_status.lock); - acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0); - c3_cpu_status.count--; + if ( c3_cpu_status.count-- == num_online_cpus() ) + acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0); spin_unlock(&c3_cpu_status.lock); } @@ -821,12 +821,10 @@ static int check_cx(struct acpi_processo return -EINVAL; /* All the logic here assumes flags.bm_check is same across all CPUs */ - if ( bm_check_flag == -1 ) + if ( bm_check_flag < 0 ) { /* Determine whether bm_check is needed based on CPU */ acpi_processor_power_init_bm_check(&(power->flags)); - bm_check_flag = power->flags.bm_check; - bm_control_flag = power->flags.bm_control; } else { @@ -853,14 +851,13 @@ static int check_cx(struct acpi_processo } } /* - * On older chipsets, BM_RLD needs to be set - * in order for Bus Master activity to wake the - * system from C3. Newer chipsets handle DMA - * during C3 automatically and BM_RLD is a NOP. - * In either case, the proper way to - * handle BM_RLD is to set it and leave it set. + * On older chipsets, BM_RLD needs to be set in order for Bus + * Master activity to wake the system from C3, hence + * acpi_set_register() is always being called once below. Newer + * chipsets handle DMA during C3 automatically and BM_RLD is a + * NOP. In either case, the proper way to handle BM_RLD is to + * set it and leave it set. */ - acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 1); } else { @@ -875,7 +872,13 @@ static int check_cx(struct acpi_processo " for C3 to be enabled on SMP systems\n")); return -EINVAL; } - acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 0); + } + + if ( bm_check_flag < 0 ) + { + bm_check_flag = power->flags.bm_check; + bm_control_flag = power->flags.bm_control; + acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, bm_check_flag); } break;