linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] power_end event (Resend)
@ 2010-06-09 13:57 Robert Schöne
  2010-06-09 17:05 ` Arjan van de Ven
  0 siblings, 1 reply; 4+ messages in thread
From: Robert Schöne @ 2010-06-09 13:57 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Arjan van de Ven, Thomas Gleixner
  Cc: linux-kernel

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.

Signed-off-by: Robert Schöne <robert.schoene@tu-dresden.de>

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

diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index f6c6266..134f5f9 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -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();
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 17cb329..01c55ed 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -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. */





^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] power_end event (Resend)
  2010-06-09 13:57 [PATCH] power_end event (Resend) Robert Schöne
@ 2010-06-09 17:05 ` Arjan van de Ven
  2010-06-09 18:12   ` Ingo Molnar
  0 siblings, 1 reply; 4+ messages in thread
From: Arjan van de Ven @ 2010-06-09 17:05 UTC (permalink / raw)
  To: Robert Schöne
  Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, linux-kernel

On 6/9/2010 6:57 AM, Robert Schöne wrote:
> 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.
>    

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

> 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.
>
>    

the patch makes sense; Acked-by: Arjan van de Ven <arjan@linux.intel.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] power_end event (Resend)
  2010-06-09 17:05 ` Arjan van de Ven
@ 2010-06-09 18:12   ` Ingo Molnar
  0 siblings, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2010-06-09 18:12 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Robert Sch??ne, Peter Zijlstra, Thomas Gleixner, linux-kernel


* Arjan van de Ven <arjan@linux.intel.com> wrote:

> the patch makes sense; Acked-by: Arjan van de Ven <arjan@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] power_end event (Resend)
@ 2010-05-27  7:01 Robert Schöne
  0 siblings, 0 replies; 4+ messages in thread
From: Robert Schöne @ 2010-05-27  7:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Peter Zijlstra, Arjan van de Ven, Thomas Gleixner, linux-kernel

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.

Signed-off-by: Robert Schöne <robert.schoene@tu-dresden.de>

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

diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index f6c6266..134f5f9 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -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();
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 17cb329..01c55ed 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -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. */




^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-06-09 18:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-09 13:57 [PATCH] power_end event (Resend) Robert Schöne
2010-06-09 17:05 ` Arjan van de Ven
2010-06-09 18:12   ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2010-05-27  7:01 Robert Schöne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).