| Message ID | 1404293458-9799-1-git-send-email-sandeep.tripathy@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/02/2014 11:30 AM, Sandeep Tripathy wrote: > idle_exit event is the first event after a core exits > idle state. So this should be traced before local irq > is ebabled. Likewise idle_entry is the last event before > a core enters idle state. This will ease visualising the > cpu idle state from kernel traces. > > Signed-off-by: Sandeep Tripathy <sandeep.tripathy@linaro.org> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/cpuidle/cpuidle.c | 3 +++ > kernel/sched/idle.c | 4 ---- > 2 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > index 8236746..97680d0 100644 > --- a/drivers/cpuidle/cpuidle.c > +++ b/drivers/cpuidle/cpuidle.c > @@ -99,12 +99,15 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, > ktime_t time_start, time_end; > s64 diff; > > + trace_cpu_idle_rcuidle(index, dev->cpu); > time_start = ktime_get(); > > entered_state = target_state->enter(dev, drv, index); > > time_end = ktime_get(); > > + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu); > + > if (!cpuidle_state_is_coupled(dev, drv, entered_state)) > local_irq_enable(); > > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index 8f4390a..07c446a 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -141,7 +141,6 @@ static int cpuidle_idle_call(void) > &dev->cpu); > > if (!ret) { > - trace_cpu_idle_rcuidle(next_state, dev->cpu); > > /* > * Enter the idle state previously > @@ -154,9 +153,6 @@ static int cpuidle_idle_call(void) > entered_state = cpuidle_enter(drv, dev, > next_state); > > - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, > - dev->cpu); > - > if (broadcast) > clockevents_notify( > CLOCK_EVT_NOTIFY_BROADCAST_EXIT, >
On Saturday, July 05, 2014 05:45:24 PM Daniel Lezcano wrote: > On 07/02/2014 11:30 AM, Sandeep Tripathy wrote: > > idle_exit event is the first event after a core exits > > idle state. So this should be traced before local irq > > is ebabled. Likewise idle_entry is the last event before > > a core enters idle state. This will ease visualising the > > cpu idle state from kernel traces. > > > > Signed-off-by: Sandeep Tripathy <sandeep.tripathy@linaro.org> > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> I'm going to take this for 3.17. Peter, will there be any problems with it if I take it? Rafael > > --- > > drivers/cpuidle/cpuidle.c | 3 +++ > > kernel/sched/idle.c | 4 ---- > > 2 files changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > > index 8236746..97680d0 100644 > > --- a/drivers/cpuidle/cpuidle.c > > +++ b/drivers/cpuidle/cpuidle.c > > @@ -99,12 +99,15 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, > > ktime_t time_start, time_end; > > s64 diff; > > > > + trace_cpu_idle_rcuidle(index, dev->cpu); > > time_start = ktime_get(); > > > > entered_state = target_state->enter(dev, drv, index); > > > > time_end = ktime_get(); > > > > + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu); > > + > > if (!cpuidle_state_is_coupled(dev, drv, entered_state)) > > local_irq_enable(); > > > > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > > index 8f4390a..07c446a 100644 > > --- a/kernel/sched/idle.c > > +++ b/kernel/sched/idle.c > > @@ -141,7 +141,6 @@ static int cpuidle_idle_call(void) > > &dev->cpu); > > > > if (!ret) { > > - trace_cpu_idle_rcuidle(next_state, dev->cpu); > > > > /* > > * Enter the idle state previously > > @@ -154,9 +153,6 @@ static int cpuidle_idle_call(void) > > entered_state = cpuidle_enter(drv, dev, > > next_state); > > > > - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, > > - dev->cpu); > > - > > if (broadcast) > > clockevents_notify( > > CLOCK_EVT_NOTIFY_BROADCAST_EXIT, > > > > >
On Mon, Jul 14, 2014 at 09:47:46PM +0200, Rafael J. Wysocki wrote: > On Saturday, July 05, 2014 05:45:24 PM Daniel Lezcano wrote: > > On 07/02/2014 11:30 AM, Sandeep Tripathy wrote: > > > idle_exit event is the first event after a core exits > > > idle state. This > > > So this should be traced before local irq > > > is ebabled. does not follow, interrupt state does not correlate to events or not. > > > Likewise idle_entry is the last event before > > > a core enters idle state. This will ease visualising the > > > cpu idle state from kernel traces. > > > > > > Signed-off-by: Sandeep Tripathy <sandeep.tripathy@linaro.org> > > > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > I'm going to take this for 3.17. > > Peter, will there be any problems with it if I take it? don't think so, if there's anything, I'll fix it up. > > > --- > > > drivers/cpuidle/cpuidle.c | 3 +++ > > > kernel/sched/idle.c | 4 ---- > > > 2 files changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > > > index 8236746..97680d0 100644 > > > --- a/drivers/cpuidle/cpuidle.c > > > +++ b/drivers/cpuidle/cpuidle.c > > > @@ -99,12 +99,15 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, > > > ktime_t time_start, time_end; > > > s64 diff; > > > > > > + trace_cpu_idle_rcuidle(index, dev->cpu); > > > time_start = ktime_get(); > > > > > > entered_state = target_state->enter(dev, drv, index); > > > > > > time_end = ktime_get(); > > > > > > + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu); If someone were to instrument ktime_get() the changelog is obviously fail. Now I appreciate that instrumenting the timing infrastructure might be challenging .. :-) And I suppose the 'assumption' is that the target_state->enter() function does not entail 'tracing' ? Is it made sure that these functions do not generate __mcount or other function tracer stubs etc. ?
On Mon, 14 Jul 2014 22:07:16 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > And I suppose the 'assumption' is that the target_state->enter() > function does not entail 'tracing' ? Is it made sure that these > functions do not generate __mcount or other function tracer stubs etc. ? Lets not confuse events with function tracing. Function tracing has its own infrastructure and is made to trace details of things like target_state->enter() and such. And function tracing can trace even outside of rcu scope, which trace events do not. When people want to trace events, that assumption is just trace events, not function tracing. -- Steve
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 8236746..97680d0 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -99,12 +99,15 @@ int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, ktime_t time_start, time_end; s64 diff; + trace_cpu_idle_rcuidle(index, dev->cpu); time_start = ktime_get(); entered_state = target_state->enter(dev, drv, index); time_end = ktime_get(); + trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu); + if (!cpuidle_state_is_coupled(dev, drv, entered_state)) local_irq_enable(); diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 8f4390a..07c446a 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -141,7 +141,6 @@ static int cpuidle_idle_call(void) &dev->cpu); if (!ret) { - trace_cpu_idle_rcuidle(next_state, dev->cpu); /* * Enter the idle state previously @@ -154,9 +153,6 @@ static int cpuidle_idle_call(void) entered_state = cpuidle_enter(drv, dev, next_state); - trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, - dev->cpu); - if (broadcast) clockevents_notify( CLOCK_EVT_NOTIFY_BROADCAST_EXIT,
idle_exit event is the first event after a core exits idle state. So this should be traced before local irq is ebabled. Likewise idle_entry is the last event before a core enters idle state. This will ease visualising the cpu idle state from kernel traces. Signed-off-by: Sandeep Tripathy <sandeep.tripathy@linaro.org> --- drivers/cpuidle/cpuidle.c | 3 +++ kernel/sched/idle.c | 4 ---- 2 files changed, 3 insertions(+), 4 deletions(-)