acpi c-states: Fix multiply C-states name disturbance

There are possible multiply C-states for special type of ACPI C-state, such as
ACPI C3, processor C3,C6,C7 are possible all mapped to ACPI C3.

In /sys/devices/system/cpu/cpu0/cpuidle/stateXX, state<number> will increase with number of validate C-states. But its C-state name under stateXX will also increase with number of validate C-state.

Actually, C-state name under stateXX should be type of ACPI C-state which we can refer to /proc/acpi/processor/CPU0/power type[C-state].

This patch unify that all the multiply C-states to one special ACPI C-state.

---

index 302d656..a94b4d7 100644
@@ -1086,7 +1086,6 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 #endif
 		cpuidle_set_statedata(state, cx);
 
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
 		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
 		state->exit_latency = cx->latency;
 		state->target_residency = cx->latency * latency_factor;

@@ -1099,6 +1098,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			if (cx->entry_method == ACPI_CSTATE_FFH)
 				state->flags |= CPUIDLE_FLAG_TIME_VALID;
 
+			snprintf(state->name, CPUIDLE_NAME_LEN, "C%d",
+					ACPI_STATE_C1);
 			state->enter = acpi_idle_enter_c1;
 			dev->safe_state = state;
 			break;

@@ -1106,6 +1107,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			case ACPI_STATE_C2:
 			state->flags |= CPUIDLE_FLAG_BALANCED;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
+			snprintf(state->name, CPUIDLE_NAME_LEN, "C%d",
+				ACPI_STATE_C2);
 			state->enter = acpi_idle_enter_simple;
 			dev->safe_state = state;
 			break;

@@ -1114,6 +1117,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			state->flags |= CPUIDLE_FLAG_DEEP;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
 			state->flags |= CPUIDLE_FLAG_CHECK_BM;
+			snprintf(state->name, CPUIDLE_NAME_LEN, "C%d",
+				ACPI_STATE_C3);
 			state->enter = pr->flags.bm_check ?
 					acpi_idle_enter_bm :
 					acpi_idle_enter_simple;

Ack the idea of this patch.

I don't quite agree with the comment
"Actually, C-state name under stateXX should be type of ACPI"

cpuidle is more generic than ACPI and C-state name under stateXX can be anything that the driver wants to be.

I agree with your point though that today there is no mapping for ACPI state in cpuidle sysfs output and it will be nice to add It.

However, I think it is better to have

There are possible multiply C-states for special type of ACPI C-state, such as
ACPI C3, processor C3,C6,C7 are possible all mapped to ACPI C3.

In /sys/devices/system/cpu/cpu0/cpuidle/stateXX, state<number> will increase with number of validate C-states. But its C-state name under stateXX will also increase with number of validate C-state.

This patch unify that all the multiply C-states to one special ACPI C-state.

---

index 302d656..f14785d 100644
@@ -1086,7 +1086,6 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 #endif
 		cpuidle_set_statedata(state, cx);
 
-		snprintf(state->name, CPUIDLE_NAME_LEN, "C%d", i);
 		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
 		state->exit_latency = cx->latency;
 		state->target_residency = cx->latency * latency_factor;

@@ -1099,6 +1098,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			if (cx->entry_method == ACPI_CSTATE_FFH)
 				state->flags |= CPUIDLE_FLAG_TIME_VALID;
 
+			snprintf(state->name, CPUIDLE_NAME_LEN, "ACPI C%d",
+					ACPI_STATE_C1);
 			state->enter = acpi_idle_enter_c1;
 			dev->safe_state = state;
 			break;

@@ -1106,6 +1107,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			case ACPI_STATE_C2:
 			state->flags |= CPUIDLE_FLAG_BALANCED;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
+			snprintf(state->name, CPUIDLE_NAME_LEN, "ACPI C%d",
+				ACPI_STATE_C2);
 			state->enter = acpi_idle_enter_simple;
 			dev->safe_state = state;
 			break;

@@ -1114,6 +1117,8 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 			state->flags |= CPUIDLE_FLAG_DEEP;
 			state->flags |= CPUIDLE_FLAG_TIME_VALID;
 			state->flags |= CPUIDLE_FLAG_CHECK_BM;
+			snprintf(state->name, CPUIDLE_NAME_LEN, "ACPI C%d",
+				ACPI_STATE_C3);
 			state->enter = pr->flags.bm_check ?
 					acpi_idle_enter_bm :
 					acpi_idle_enter_simple;

Tickless is disabled by nohz=off, which is used in OSVs. But in current kernel, if tickless is disabled, the timer irq0 will not increase. Because the timer event handler should be tick_handle_periodic, but actually event handler keep as tick_handle_oneshot_broadcast which is used in tickless. The root cause is that it is default to enable high resolution timer which will force to oneshot
broadcast mode.

This patch add tickless enable check before enable high resolution timer

On Nehalem-EX:

Before the patch:
linux-a25n:~ # cat /proc/interrupts | grep timer
0: 334 0 0 0 0 0 ....
LOC: 192248 193931 193851 184441 193803 193625 ....

After the patch:
cat /proc/interrupts | grep timer
0: 223788 0 0 0 0 0 ....
LOC: 13081 238407 238452 229405 238298 235688 ....

---

index f992762..a515bed 100644
@@ -815,7 +815,7 @@ int tick_check_oneshot_change(int allow_nohz)
 	if (!timekeeping_valid_for_hres() || !tick_is_oneshot_available())
 		return 0;
 
-	if (!allow_nohz)
+	if (!allow_nohz && tick_nohz_enabled)
 		return 1;
 
 	tick_nohz_switch_to_nohz();

it is? I doubt anyone wants to normally disable tickless, the power cost is just too high....

why is this a problem?

using local apic in one shot mode is not a problem, I'd in fact call it a feature...

Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

lapic yes, broadcast not so much.

He mentioned he was using Nehelem-EX which is known broken in that it stops the lapic in deeper C states and thus reverts to HPET broadcast since there's not enough HPET timers to go around.

Something like processor.max_cstate=2 should fix that IIRC.

Jens ran into this on his shiny new box.

Disabling nohz is funny indeed, one wonders why he does that.

I do not have real case for it. I look at nohz=off because
1. OSV has such release notes to explain how to disable tickless.
2. with some OSV config + latest kernel, it will result in kernel oops and it config disable tickless. I have reported it to acpi maillist before.

Thanks.

-Youquan

tickless is normally used but it is possible to use for some special users, tickless is disabled by nohz=off kernel option, which is address in OSV's release notes.

Two reasons:
R1:
In my mind, tick is period timer with frequent of HZ. but timer irq0 is actually be an oneshot mode timer.
But in real watch, it is keep not increase for long time. but it suddenly increase 5000~10000 times in a seconds.

R2:
Tickless disable, change cpuidle driver to ladder governor(menu does not work at all).

Run powertop, if apply my patch, the c-state residency has > 10% improvement than current kernel.

But in current kernel, local apic timer is in periodic mode with frequencey of HZ when tickless is disabled.