[RFC PATCH] use acpi_idle_enter_simple if bm_check && !.bm_control

[RFC PATCH] use acpi_idle_enter_simple if bm_check && !.bm_control

[Luming Yu]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

Hi there,

I came across acpi processor idle driver, noticed that we can cut a
bit overhead at C3 entry that can improve C3 residency a bit,
especially when it has similar kernel config as old RHEL 5 kernel
(2.6.18) on systems with as many as 64 logical CPUs. The point of this
patch is bm_sts is an optional bit. It never returns 1 on systems with
bm_check && !bm_control I tested. Instead, I have observed lower C3
residency due to accessing bm_sts and relevant code on a system with
64 logical CPUs with HZ=1000. Please review. If make sense, please
apply.

Ps. The patch is enclosed in attachment. The in-lined one is c&p of it
for reading.

Thanks,
Luming

Signed-off-by: Yu Luming <luming.yu@intel.com>

 processor_idle.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

 processor_idle.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
[root@hp-core tmp]# cat 12.patch
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 66393d5..ee72bbb 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -895,7 +895,7 @@ static int acpi_idle_enter_simple(struct
cpuidle_device *dev,
 	 */
 	lapic_timer_state_broadcast(pr, cx, 1);

-	if (cx->type == ACPI_STATE_C3)
+	if (cx->type == ACPI_STATE_C3 && !pr->flags.bm_check)
 		ACPI_FLUSH_CPU_CACHE();

 	kt1 = ktime_get_real();
@@ -1110,6 +1110,8 @@ static int acpi_processor_setup_cpuidle(struct
acpi_processor *pr)
 			state->enter = pr->flags.bm_check ?
 					acpi_idle_enter_bm :
 					acpi_idle_enter_simple;
+			if (pr->flags.bm_check && !pr->flags.bm_control)
+				state->enter = acpi_idle_enter_simple;
 			break;
 		}

[-- Attachment #2: 12.patch --]
[-- Type: application/octet-stream, Size: 754 bytes --]

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 66393d5..ee72bbb 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -895,7 +895,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 	 */
 	lapic_timer_state_broadcast(pr, cx, 1);
 
-	if (cx->type == ACPI_STATE_C3)
+	if (cx->type == ACPI_STATE_C3 && !pr->flags.bm_check)
 		ACPI_FLUSH_CPU_CACHE();
 
 	kt1 = ktime_get_real();
@@ -1110,6 +1110,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			state->enter = pr->flags.bm_check ?
 					acpi_idle_enter_bm :
 					acpi_idle_enter_simple;
+			if (pr->flags.bm_check && !pr->flags.bm_control)
+				state->enter = acpi_idle_enter_simple;
 			break;
 		}
 

Re: [RFC PATCH] use acpi_idle_enter_simple if bm_check && !.bm_control

[Matthew Garrett]

On Tue, Nov 10, 2009 at 02:12:41PM +0800, Luming Yu wrote:

> I came across acpi processor idle driver, noticed that we can cut a
> bit overhead at C3 entry that can improve C3 residency a bit,
> especially when it has similar kernel config as old RHEL 5 kernel
> (2.6.18) on systems with as many as 64 logical CPUs. The point of this
> patch is bm_sts is an optional bit. It never returns 1 on systems with
> bm_check && !bm_control I tested. Instead, I have observed lower C3
> residency due to accessing bm_sts and relevant code on a system with
> 64 logical CPUs with HZ=1000. Please review. If make sense, please
> apply.

Saves 40W or so on a dual-socket Nehalem system here. Is there a reason 
it wasn't picked up?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH] use acpi_idle_enter_simple if bm_check && !.bm_control

[Len Brown]

On Mon, 19 Apr 2010, Matthew Garrett wrote:

> On Tue, Nov 10, 2009 at 02:12:41PM +0800, Luming Yu wrote:
> 
> > I came across acpi processor idle driver, noticed that we can cut a
> > bit overhead at C3 entry that can improve C3 residency a bit,
> > especially when it has similar kernel config as old RHEL 5 kernel
> > (2.6.18) on systems with as many as 64 logical CPUs. The point of this
> > patch is bm_sts is an optional bit. It never returns 1 on systems with
> > bm_check && !bm_control I tested. Instead, I have observed lower C3
> > residency due to accessing bm_sts and relevant code on a system with
> > 64 logical CPUs with HZ=1000. Please review. If make sense, please
> > apply.
> 
> Saves 40W or so on a dual-socket Nehalem system here. Is there a reason 
> it wasn't picked up?

The reason it wasn't picked up is because it assumed that the need
for checking BM_STS is identical to the ability to use ARB_DIS.

However, the two are entirely independent.  The result is that
with the patch we would stop checking BM_STS for every Core2 Duo
system -- which would be incorrect.

thanks,
Len Brown, Intel Open Source Technology Center