Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Len Brown]

 So, I suggest that if neither Len nor Arjan reappear shortly, people can
 send CPUidle patches to me.


/me reappears

this series is in my tree now, and I'll be poking at it a bit tomorrow.
If everything is happy I'll send it for 3.4.


thanks,
-Len
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Len Brown]

  If the display tools can only handle 3 characters,
  then why not have them simply use the 1st 3 characters
  of the existing name field?

 You mean use:
 C3 NHM
 instead of
 NHM-C3
 for intel_idle?

Yes.
is there a reason that would be a problem?

If only 3 characters are useful,
then I'd rather see you simly do this:

#define CPUIDLE_NAME_LEN 3

 Then userspace has to parse the two informations glued
 together, get rid of the whitespace, etc.

what two informations?
why is kernel bloat the instrument of choice
to avoid the expense of string truncation
in user-space tools?

 But by sysfs convention a separate file must be used
 if two data are passed to userspace which is the case here.

what two data?

It is fine for a string to include space characters.

  If that is not unique,
  then re-arrange the strings so that it is unique...

 See above, it would unnecessarily make life hard for
 userspace.
 Afaik sysfs entries do not consume resources as long as
 they do not get accessed.

they clutter the source code,
they create  an additional name for something
that arguably already has 3 names --
the state #, the name, and the description.
I'd rather delete a few of the fields than add a 4th...

 Of course the ACPI part of this patch will not apply,
  as it depends on patch 1 in this series, which was erroneous.
  For ACPI, the existing name field is already fine,
  as C%d fits into 3 characters.

 For acpi_idle only it would work, but this is (nearly) the only
 one.
 For example for sh arch:
 name: SuperH
 SH SuperH contains two data and should get split up into:
 name: SuperH
 abbr: SH

acpi/arch/sh/kernel/cpu/shmobile/cpuidle.c
already uses state-name = C0, C1, C2.

Just use them and be done.

 By convention the first characters of description could be
 used, but you would again end up in userspace parsing and
 double info in one sysfs file.
 
 I hope that's enough for convincing, I'd really like to go
 the .abbr way. Do you give me an acked-by for that one?

No.

We should not invent an additional abbreviation field.
The tools should use the existing name field.
If the existing name field is not sufficient, then
the states should simply be enumerated and numbers
be shown in the GUI.

thanks,
Len Brown, Intel Open Source Technology Center



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Len Brown]

Applied to idle-test

git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-idle-2.6.git idle-test

Thomas,
Please test this branch, and base your incremental patches on it.

thanks,
Len Brown, Intel Open Source Technology Center


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 12/12] cpuidle/x86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Len Brown]

From: Thomas Renninger tr...@suse.de

Currently intel_idle and acpi_idle driver show double cpu_idle exit idle
events - this patch fixes it and makes cpu_idle events throwing less complex.

It also introduces cpu_idle events for all architectures which use
the cpuidle subsystem, namely:
  - arch/arm/mach-at91/cpuidle.c
  - arch/arm/mach-davinci/cpuidle.c
  - arch/arm/mach-kirkwood/cpuidle.c
  - arch/arm/mach-omap2/cpuidle34xx.c
  - arch/drivers/acpi/processor_idle.c (for all cases, not only mwait)
  - arch/x86/kernel/process.c (did throw events before, but was a mess)
  - drivers/idle/intel_idle.c (did throw events before)

Convention should be:
Fire cpu_idle events inside the current pm_idle function (not somewhere
down the the callee tree) to keep things easy.

Current possible pm_idle functions in X86:
c1e_idle, poll_idle, cpuidle_idle_call, mwait_idle, default_idle
- this is really easy is now.

This affects userspace:
The type field of the cpu_idle power event can now direclty get
mapped to:
/sys/devices/system/cpu/cpuX/cpuidle/stateX/{name,desc,usage,time,...}
instead of throwing very CPU/mwait specific values.
This change is not visible for the intel_idle driver.
For the acpi_idle driver it should only be visible if the vendor
misses out C-states in his BIOS.
Another (perf timechart) patch reads out cpuidle info of cpu_idle
events from:
/sys/.../cpuidle/stateX/*, then the cpuidle events are mapped
to the correct C-/cpuidle state again, even if e.g. vendors miss
out C-states in their BIOS and for example only export C1 and C3.
- everything is fine.

Signed-off-by: Thomas Renninger tr...@suse.de
CC: Robert Schoene robert.scho...@tu-dresden.de
CC: Jean Pihet j-pi...@ti.com
CC: Arjan van de Ven ar...@linux.intel.com
CC: Ingo Molnar mi...@elte.hu
CC: Frederic Weisbecker fweis...@gmail.com
CC: linux...@lists.linux-foundation.org
CC: linux-a...@vger.kernel.org
CC: linux-ker...@vger.kernel.org
CC: linux-perf-us...@vger.kernel.org
CC: linux-omap@vger.kernel.org
Signed-off-by: Len Brown len.br...@intel.com
---
 arch/x86/kernel/process.c|6 --
 arch/x86/kernel/process_32.c |4 
 arch/x86/kernel/process_64.c |6 --
 drivers/cpuidle/cpuidle.c|   10 --
 drivers/idle/intel_idle.c|2 --
 5 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 09c08a1..67e96e6 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -386,6 +386,8 @@ void default_idle(void)
else
local_irq_enable();
current_thread_info()-status |= TS_POLLING;
+   trace_power_end(smp_processor_id());
+   trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
} else {
local_irq_enable();
/* loop is done by the caller */
@@ -443,8 +445,6 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);
  */
 void mwait_idle_with_hints(unsigned long ax, unsigned long cx)
 {
-   trace_power_start(POWER_CSTATE, (ax4)+1, smp_processor_id());
-   trace_cpu_idle((ax4)+1, smp_processor_id());
if (!need_resched()) {
if (cpu_has(__this_cpu_ptr(cpu_info), 
X86_FEATURE_CLFLUSH_MONITOR))
clflush((void *)current_thread_info()-flags);
@@ -471,6 +471,8 @@ static void mwait_idle(void)
__sti_mwait(0, 0);
else
local_irq_enable();
+   trace_power_end(smp_processor_id());
+   trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
} else
local_irq_enable();
 }
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 4b9befa..8d12878 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -57,8 +57,6 @@
 #include asm/syscalls.h
 #include asm/debugreg.h
 
-#include trace/events/power.h
-
 asmlinkage void ret_from_fork(void) __asm__(ret_from_fork);
 
 /*
@@ -113,8 +111,6 @@ void cpu_idle(void)
stop_critical_timings();
pm_idle();
start_critical_timings();
-   trace_power_end(smp_processor_id());
-   trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
}
tick_nohz_restart_sched_tick();
preempt_enable_no_resched();
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 4c818a7..bd387e8 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -51,8 +51,6 @@
 #include asm/syscalls.h
 #include asm/debugreg.h
 
-#include trace/events/power.h
-
 asmlinkage extern void ret_from_fork(void);
 
 DEFINE_PER_CPU(unsigned long, old_rsp);
@@ -141,10 +139,6 @@ void cpu_idle(void)
pm_idle();
start_critical_timings();
 
-   trace_power_end(smp_processor_id());
-   trace_cpu_idle

Re: [PATCH 3/9] X86/perf: fix power:cpu_idle double end events and throw cpu_idle events from the cpuidle layer

[Len Brown]

I'm happy to see the trace point move up into cpuidle from intel_idle.

If somebody is picking this up in a perf tree,
Acked-by: Len Brown len.br...@intel.com

else I can put it in the idle tree, let me know.

thanks,
Len Brown, Intel Open Source Technology Center

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] cpuidle: Introduce .abbr (abbrevation) for cpuidle states

[Len Brown]

I'm not fond of inventing a new 3-character abbreviation field
for every state because display tools can't handle the existing
16-character name field.

If the display tools can only handle 3 characters,
then why not have them simply use the 1st 3 characters
of the existing name field?  If that is not unique,
then re-arrange the strings so that it is unique...

Of course the ACPI part of this patch will not apply,
as it depends on patch 1 in this series, which was erroneous.
For ACPI, the existing name field is already fine,
as C%d fits into 3 characters.

thanks,
Len Brown, Intel Open Source Technology Center

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html