power_end event (Resend)

Original Mail was sent at 2010/05/14 10:38:43 CEST

Hi,
I reported the power_end tracing problem earlier this year
(http://lkml.org/lkml/2010/2/24/79) and sent a patch which worked for my system. However this patch would have not worked on other systems (as for example Arjans). It would had lead to a double posting of these events.

However. Here's a diff that should fix the problem on the correct spot.

The reason that it worked for Arjan and not for me is that his system uses drivers/acpi/processor_idle.c when idling, mine uses the cpu_idle thread from arch/x86/kernel/process_64.c.
A comparable idle thread also exists for 32 bit x86, so I added it in process_32.c too.

However, is there any standard about where to report the start and end events? Currently it's the idle routine, which creates the power_start event, the routine which calls the idle_routine on the other hand creates the power_end event.

For these patches, I'm not sure whether the power_end event should even be reported. On kernels, which use the repnop loop when idling, there won't be a switch to another c-state and therefore no power_start event, the power_end event could belong to. Would that be a problem? If it would, the only way to fix this would be to move the power_end events into the idle routines, since cpu_idle is dumb and does not know whats behind pm_idle.

Robert

PS: I hand over the smp_processor_id() to power_end since i hope that the changed interface defined in http://lkml.org/lkml/2010/4/27/271 will be part of the standard tree one day. As long as this new interface is not included in the standard tree, this patch won't make no harm. When it will be included, this code does not have to be touched anymore.

 arch/x86/kernel/process_32.c |    4 ++++
 arch/x86/kernel/process_64.c |    5 +++++
 2 files changed, 9 insertions(+), 0 deletions(-)

index f6c6266..134f5f9 100644
@@ -58,6 +58,8 @@
 #include <asm/ds.h>
 #include <asm/debugreg.h>
 
+#include <trace/events/power.h> 
+
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 
 /*

@@ -112,6 +114,8 @@ void cpu_idle(void)
 			stop_critical_timings();
 			pm_idle();
 			start_critical_timings();
+
+			trace_power_end(smp_processor_id());
 		}
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();

index 17cb329..01c55ed 100644
@@ -52,6 +52,8 @@
 #include <asm/ds.h>
 #include <asm/debugreg.h>
 
+#include <trace/events/power.h> 
+
 asmlinkage extern void ret_from_fork(void);
 
 DEFINE_PER_CPU(unsigned long, old_rsp);

@@ -139,6 +141,9 @@ void cpu_idle(void)
 			stop_critical_timings();
 			pm_idle();
 			start_critical_timings();
+
+			trace_power_end(smp_processor_id());
+
 			/* In many cases the interrupt that ended idle
 			   has already called exit_idle. But some idle
 			   loops can be woken up without interrupt. */

only the actual idle routine knows what C state it goes in; there's no central way for that really.

the patch makes sense; Acked-by: Arjan van de Ven <[email protected]>

Great!

Robert, mind resending the patch with Arjan's ack included and with a meaningful changelog added as well, describing the problem and the solution.

Thanks,

Ingo