diff mbox

[1/5] intel_pstate: Add tsc collection and keep previous target pstate. Add both to trace.

Message ID 1428811830-15006-2-git-send-email-dsmythies@telus.net (mailing list archive)
State Accepted, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Doug Smythies April 12, 2015, 4:10 a.m. UTC
The intel_pstate driver is difficult to debug and investigate without tsc.
Also, it is likely use of tsc, and some version of C0 percentage,
will be re-introdcued in futute.
There have also been occasions where it is desirebale to know, and
confirm, the previous target pstate.
This patch brings back tsc, adds previous target pstate,
and adds both to the trace data collection.

Signed-off-by: Doug Smythies <dsmythies@telus.net>
---
 drivers/cpufreq/intel_pstate.c | 31 +++++++++++++++++++++----------
 include/trace/events/power.h   | 25 +++++++++++++++++--------
 2 files changed, 38 insertions(+), 18 deletions(-)

Comments

Kristen Carlson Accardi April 29, 2015, 4:57 p.m. UTC | #1
On Sat, 11 Apr 2015 21:10:26 -0700
Doug Smythies <doug.smythies@gmail.com> wrote:

> The intel_pstate driver is difficult to debug and investigate without tsc.
> Also, it is likely use of tsc, and some version of C0 percentage,
> will be re-introdcued in futute.
> There have also been occasions where it is desirebale to know, and
> confirm, the previous target pstate.
> This patch brings back tsc, adds previous target pstate,
> and adds both to the trace data collection.
> 
> Signed-off-by: Doug Smythies <dsmythies@telus.net>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  drivers/cpufreq/intel_pstate.c | 31 +++++++++++++++++++++----------
>  include/trace/events/power.h   | 25 +++++++++++++++++--------
>  2 files changed, 38 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 872c577..f181ce5 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -67,6 +67,7 @@ struct sample {
>  	int32_t core_pct_busy;
>  	u64 aperf;
>  	u64 mperf;
> +	u64 tsc;
>  	int freq;
>  	ktime_t time;
>  };
> @@ -108,6 +109,7 @@ struct cpudata {
>  	ktime_t last_sample_time;
>  	u64	prev_aperf;
>  	u64	prev_mperf;
> +	u64	prev_tsc;
>  	struct sample sample;
>  };
>  
> @@ -725,23 +727,28 @@ static inline void intel_pstate_sample(struct cpudata *cpu)
>  {
>  	u64 aperf, mperf;
>  	unsigned long flags;
> +	u64 tsc;
>  
>  	local_irq_save(flags);
>  	rdmsrl(MSR_IA32_APERF, aperf);
>  	rdmsrl(MSR_IA32_MPERF, mperf);
> +	tsc = native_read_tsc();
>  	local_irq_restore(flags);
>  
>  	cpu->last_sample_time = cpu->sample.time;
>  	cpu->sample.time = ktime_get();
>  	cpu->sample.aperf = aperf;
>  	cpu->sample.mperf = mperf;
> +	cpu->sample.tsc =  tsc;
>  	cpu->sample.aperf -= cpu->prev_aperf;
>  	cpu->sample.mperf -= cpu->prev_mperf;
> +	cpu->sample.tsc -= cpu->prev_tsc;
>  
>  	intel_pstate_calc_busy(cpu);
>  
>  	cpu->prev_aperf = aperf;
>  	cpu->prev_mperf = mperf;
> +	cpu->prev_tsc = tsc;
>  }
>  
>  static inline void intel_hwp_set_sample_time(struct cpudata *cpu)
> @@ -806,6 +813,10 @@ static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
>  	int32_t busy_scaled;
>  	struct _pid *pid;
>  	signed int ctl;
> +	int from;
> +	struct sample *sample;
> +
> +	from = cpu->pstate.current_pstate;
>  
>  	pid = &cpu->pid;
>  	busy_scaled = intel_pstate_get_scaled_busy(cpu);
> @@ -814,6 +825,16 @@ static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
>  
>  	/* Negative values of ctl increase the pstate and vice versa */
>  	intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
> +
> +	sample = &cpu->sample;
> +	trace_pstate_sample(fp_toint(sample->core_pct_busy),
> +		fp_toint(busy_scaled),
> +		from,
> +		cpu->pstate.current_pstate,
> +		sample->mperf,
> +		sample->aperf,
> +		sample->tsc,
> +		sample->freq);
>  }
>  
>  static void intel_hwp_timer_func(unsigned long __data)
> @@ -827,21 +848,11 @@ static void intel_hwp_timer_func(unsigned long __data)
>  static void intel_pstate_timer_func(unsigned long __data)
>  {
>  	struct cpudata *cpu = (struct cpudata *) __data;
> -	struct sample *sample;
>  
>  	intel_pstate_sample(cpu);
>  
> -	sample = &cpu->sample;
> -
>  	intel_pstate_adjust_busy_pstate(cpu);
>  
> -	trace_pstate_sample(fp_toint(sample->core_pct_busy),
> -			fp_toint(intel_pstate_get_scaled_busy(cpu)),
> -			cpu->pstate.current_pstate,
> -			sample->mperf,
> -			sample->aperf,
> -			sample->freq);
> -
>  	intel_pstate_set_sample_time(cpu);
>  }
>  
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index d19840b..630d1e5 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -42,45 +42,54 @@ TRACE_EVENT(pstate_sample,
>  
>  	TP_PROTO(u32 core_busy,
>  		u32 scaled_busy,
> -		u32 state,
> +		u32 from,
> +		u32 to,
>  		u64 mperf,
>  		u64 aperf,
> +		u64 tsc,
>  		u32 freq
>  		),
>  
>  	TP_ARGS(core_busy,
>  		scaled_busy,
> -		state,
> +		from,
> +		to,
>  		mperf,
>  		aperf,
> +		tsc,
>  		freq
>  		),
>  
>  	TP_STRUCT__entry(
>  		__field(u32, core_busy)
>  		__field(u32, scaled_busy)
> -		__field(u32, state)
> +		__field(u32, from)
> +		__field(u32, to)
>  		__field(u64, mperf)
>  		__field(u64, aperf)
> +		__field(u64, tsc)
>  		__field(u32, freq)
> -
> -	),
> +		),
>  
>  	TP_fast_assign(
>  		__entry->core_busy = core_busy;
>  		__entry->scaled_busy = scaled_busy;
> -		__entry->state = state;
> +		__entry->from = from;
> +		__entry->to = to;
>  		__entry->mperf = mperf;
>  		__entry->aperf = aperf;
> +		__entry->tsc = tsc;
>  		__entry->freq = freq;
>  		),
>  
> -	TP_printk("core_busy=%lu scaled=%lu state=%lu mperf=%llu aperf=%llu freq=%lu ",
> +	TP_printk("core_busy=%lu scaled=%lu from=%lu to=%lu mperf=%llu aperf=%llu tsc=%llu freq=%lu ",
>  		(unsigned long)__entry->core_busy,
>  		(unsigned long)__entry->scaled_busy,
> -		(unsigned long)__entry->state,
> +		(unsigned long)__entry->from,
> +		(unsigned long)__entry->to,
>  		(unsigned long long)__entry->mperf,
>  		(unsigned long long)__entry->aperf,
> +		(unsigned long long)__entry->tsc,
>  		(unsigned long)__entry->freq
>  		)
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki May 4, 2015, 11:34 p.m. UTC | #2
On Wednesday, April 29, 2015 09:57:07 AM Kristen Carlson Accardi wrote:
> On Sat, 11 Apr 2015 21:10:26 -0700
> Doug Smythies <doug.smythies@gmail.com> wrote:
> 
> > The intel_pstate driver is difficult to debug and investigate without tsc.
> > Also, it is likely use of tsc, and some version of C0 percentage,
> > will be re-introdcued in futute.
> > There have also been occasions where it is desirebale to know, and
> > confirm, the previous target pstate.
> > This patch brings back tsc, adds previous target pstate,
> > and adds both to the trace data collection.
> > 
> > Signed-off-by: Doug Smythies <dsmythies@telus.net>
> 
> Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

Queued up for 4.2 with minor changes in the changelog/subject, thanks!

> > ---
> >  drivers/cpufreq/intel_pstate.c | 31 +++++++++++++++++++++----------
> >  include/trace/events/power.h   | 25 +++++++++++++++++--------
> >  2 files changed, 38 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index 872c577..f181ce5 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -67,6 +67,7 @@ struct sample {
> >  	int32_t core_pct_busy;
> >  	u64 aperf;
> >  	u64 mperf;
> > +	u64 tsc;
> >  	int freq;
> >  	ktime_t time;
> >  };
> > @@ -108,6 +109,7 @@ struct cpudata {
> >  	ktime_t last_sample_time;
> >  	u64	prev_aperf;
> >  	u64	prev_mperf;
> > +	u64	prev_tsc;
> >  	struct sample sample;
> >  };
> >  
> > @@ -725,23 +727,28 @@ static inline void intel_pstate_sample(struct cpudata *cpu)
> >  {
> >  	u64 aperf, mperf;
> >  	unsigned long flags;
> > +	u64 tsc;
> >  
> >  	local_irq_save(flags);
> >  	rdmsrl(MSR_IA32_APERF, aperf);
> >  	rdmsrl(MSR_IA32_MPERF, mperf);
> > +	tsc = native_read_tsc();
> >  	local_irq_restore(flags);
> >  
> >  	cpu->last_sample_time = cpu->sample.time;
> >  	cpu->sample.time = ktime_get();
> >  	cpu->sample.aperf = aperf;
> >  	cpu->sample.mperf = mperf;
> > +	cpu->sample.tsc =  tsc;
> >  	cpu->sample.aperf -= cpu->prev_aperf;
> >  	cpu->sample.mperf -= cpu->prev_mperf;
> > +	cpu->sample.tsc -= cpu->prev_tsc;
> >  
> >  	intel_pstate_calc_busy(cpu);
> >  
> >  	cpu->prev_aperf = aperf;
> >  	cpu->prev_mperf = mperf;
> > +	cpu->prev_tsc = tsc;
> >  }
> >  
> >  static inline void intel_hwp_set_sample_time(struct cpudata *cpu)
> > @@ -806,6 +813,10 @@ static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
> >  	int32_t busy_scaled;
> >  	struct _pid *pid;
> >  	signed int ctl;
> > +	int from;
> > +	struct sample *sample;
> > +
> > +	from = cpu->pstate.current_pstate;
> >  
> >  	pid = &cpu->pid;
> >  	busy_scaled = intel_pstate_get_scaled_busy(cpu);
> > @@ -814,6 +825,16 @@ static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
> >  
> >  	/* Negative values of ctl increase the pstate and vice versa */
> >  	intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
> > +
> > +	sample = &cpu->sample;
> > +	trace_pstate_sample(fp_toint(sample->core_pct_busy),
> > +		fp_toint(busy_scaled),
> > +		from,
> > +		cpu->pstate.current_pstate,
> > +		sample->mperf,
> > +		sample->aperf,
> > +		sample->tsc,
> > +		sample->freq);
> >  }
> >  
> >  static void intel_hwp_timer_func(unsigned long __data)
> > @@ -827,21 +848,11 @@ static void intel_hwp_timer_func(unsigned long __data)
> >  static void intel_pstate_timer_func(unsigned long __data)
> >  {
> >  	struct cpudata *cpu = (struct cpudata *) __data;
> > -	struct sample *sample;
> >  
> >  	intel_pstate_sample(cpu);
> >  
> > -	sample = &cpu->sample;
> > -
> >  	intel_pstate_adjust_busy_pstate(cpu);
> >  
> > -	trace_pstate_sample(fp_toint(sample->core_pct_busy),
> > -			fp_toint(intel_pstate_get_scaled_busy(cpu)),
> > -			cpu->pstate.current_pstate,
> > -			sample->mperf,
> > -			sample->aperf,
> > -			sample->freq);
> > -
> >  	intel_pstate_set_sample_time(cpu);
> >  }
> >  
> > diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> > index d19840b..630d1e5 100644
> > --- a/include/trace/events/power.h
> > +++ b/include/trace/events/power.h
> > @@ -42,45 +42,54 @@ TRACE_EVENT(pstate_sample,
> >  
> >  	TP_PROTO(u32 core_busy,
> >  		u32 scaled_busy,
> > -		u32 state,
> > +		u32 from,
> > +		u32 to,
> >  		u64 mperf,
> >  		u64 aperf,
> > +		u64 tsc,
> >  		u32 freq
> >  		),
> >  
> >  	TP_ARGS(core_busy,
> >  		scaled_busy,
> > -		state,
> > +		from,
> > +		to,
> >  		mperf,
> >  		aperf,
> > +		tsc,
> >  		freq
> >  		),
> >  
> >  	TP_STRUCT__entry(
> >  		__field(u32, core_busy)
> >  		__field(u32, scaled_busy)
> > -		__field(u32, state)
> > +		__field(u32, from)
> > +		__field(u32, to)
> >  		__field(u64, mperf)
> >  		__field(u64, aperf)
> > +		__field(u64, tsc)
> >  		__field(u32, freq)
> > -
> > -	),
> > +		),
> >  
> >  	TP_fast_assign(
> >  		__entry->core_busy = core_busy;
> >  		__entry->scaled_busy = scaled_busy;
> > -		__entry->state = state;
> > +		__entry->from = from;
> > +		__entry->to = to;
> >  		__entry->mperf = mperf;
> >  		__entry->aperf = aperf;
> > +		__entry->tsc = tsc;
> >  		__entry->freq = freq;
> >  		),
> >  
> > -	TP_printk("core_busy=%lu scaled=%lu state=%lu mperf=%llu aperf=%llu freq=%lu ",
> > +	TP_printk("core_busy=%lu scaled=%lu from=%lu to=%lu mperf=%llu aperf=%llu tsc=%llu freq=%lu ",
> >  		(unsigned long)__entry->core_busy,
> >  		(unsigned long)__entry->scaled_busy,
> > -		(unsigned long)__entry->state,
> > +		(unsigned long)__entry->from,
> > +		(unsigned long)__entry->to,
> >  		(unsigned long long)__entry->mperf,
> >  		(unsigned long long)__entry->aperf,
> > +		(unsigned long long)__entry->tsc,
> >  		(unsigned long)__entry->freq
> >  		)
> >  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 872c577..f181ce5 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -67,6 +67,7 @@  struct sample {
 	int32_t core_pct_busy;
 	u64 aperf;
 	u64 mperf;
+	u64 tsc;
 	int freq;
 	ktime_t time;
 };
@@ -108,6 +109,7 @@  struct cpudata {
 	ktime_t last_sample_time;
 	u64	prev_aperf;
 	u64	prev_mperf;
+	u64	prev_tsc;
 	struct sample sample;
 };
 
@@ -725,23 +727,28 @@  static inline void intel_pstate_sample(struct cpudata *cpu)
 {
 	u64 aperf, mperf;
 	unsigned long flags;
+	u64 tsc;
 
 	local_irq_save(flags);
 	rdmsrl(MSR_IA32_APERF, aperf);
 	rdmsrl(MSR_IA32_MPERF, mperf);
+	tsc = native_read_tsc();
 	local_irq_restore(flags);
 
 	cpu->last_sample_time = cpu->sample.time;
 	cpu->sample.time = ktime_get();
 	cpu->sample.aperf = aperf;
 	cpu->sample.mperf = mperf;
+	cpu->sample.tsc =  tsc;
 	cpu->sample.aperf -= cpu->prev_aperf;
 	cpu->sample.mperf -= cpu->prev_mperf;
+	cpu->sample.tsc -= cpu->prev_tsc;
 
 	intel_pstate_calc_busy(cpu);
 
 	cpu->prev_aperf = aperf;
 	cpu->prev_mperf = mperf;
+	cpu->prev_tsc = tsc;
 }
 
 static inline void intel_hwp_set_sample_time(struct cpudata *cpu)
@@ -806,6 +813,10 @@  static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
 	int32_t busy_scaled;
 	struct _pid *pid;
 	signed int ctl;
+	int from;
+	struct sample *sample;
+
+	from = cpu->pstate.current_pstate;
 
 	pid = &cpu->pid;
 	busy_scaled = intel_pstate_get_scaled_busy(cpu);
@@ -814,6 +825,16 @@  static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
 
 	/* Negative values of ctl increase the pstate and vice versa */
 	intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate - ctl);
+
+	sample = &cpu->sample;
+	trace_pstate_sample(fp_toint(sample->core_pct_busy),
+		fp_toint(busy_scaled),
+		from,
+		cpu->pstate.current_pstate,
+		sample->mperf,
+		sample->aperf,
+		sample->tsc,
+		sample->freq);
 }
 
 static void intel_hwp_timer_func(unsigned long __data)
@@ -827,21 +848,11 @@  static void intel_hwp_timer_func(unsigned long __data)
 static void intel_pstate_timer_func(unsigned long __data)
 {
 	struct cpudata *cpu = (struct cpudata *) __data;
-	struct sample *sample;
 
 	intel_pstate_sample(cpu);
 
-	sample = &cpu->sample;
-
 	intel_pstate_adjust_busy_pstate(cpu);
 
-	trace_pstate_sample(fp_toint(sample->core_pct_busy),
-			fp_toint(intel_pstate_get_scaled_busy(cpu)),
-			cpu->pstate.current_pstate,
-			sample->mperf,
-			sample->aperf,
-			sample->freq);
-
 	intel_pstate_set_sample_time(cpu);
 }
 
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index d19840b..630d1e5 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -42,45 +42,54 @@  TRACE_EVENT(pstate_sample,
 
 	TP_PROTO(u32 core_busy,
 		u32 scaled_busy,
-		u32 state,
+		u32 from,
+		u32 to,
 		u64 mperf,
 		u64 aperf,
+		u64 tsc,
 		u32 freq
 		),
 
 	TP_ARGS(core_busy,
 		scaled_busy,
-		state,
+		from,
+		to,
 		mperf,
 		aperf,
+		tsc,
 		freq
 		),
 
 	TP_STRUCT__entry(
 		__field(u32, core_busy)
 		__field(u32, scaled_busy)
-		__field(u32, state)
+		__field(u32, from)
+		__field(u32, to)
 		__field(u64, mperf)
 		__field(u64, aperf)
+		__field(u64, tsc)
 		__field(u32, freq)
-
-	),
+		),
 
 	TP_fast_assign(
 		__entry->core_busy = core_busy;
 		__entry->scaled_busy = scaled_busy;
-		__entry->state = state;
+		__entry->from = from;
+		__entry->to = to;
 		__entry->mperf = mperf;
 		__entry->aperf = aperf;
+		__entry->tsc = tsc;
 		__entry->freq = freq;
 		),
 
-	TP_printk("core_busy=%lu scaled=%lu state=%lu mperf=%llu aperf=%llu freq=%lu ",
+	TP_printk("core_busy=%lu scaled=%lu from=%lu to=%lu mperf=%llu aperf=%llu tsc=%llu freq=%lu ",
 		(unsigned long)__entry->core_busy,
 		(unsigned long)__entry->scaled_busy,
-		(unsigned long)__entry->state,
+		(unsigned long)__entry->from,
+		(unsigned long)__entry->to,
 		(unsigned long long)__entry->mperf,
 		(unsigned long long)__entry->aperf,
+		(unsigned long long)__entry->tsc,
 		(unsigned long)__entry->freq
 		)