[PATCH v4 0/2] Add stop callback to the cpufreq_driver interface.

[PATCH v4 0/2] Add stop callback to the cpufreq_driver interface.

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Changes: 
v3->v4
Change to only call ->stop() callback for drivers exposing the
->set_policy() callback.

v2->v3
Changed the calling of the ->stop() callback to be conditional on the
core being the last core controlled by a given policy.

v1->2
Change name of callback function added to cpufreq_driver interface.

Some drivers (intel_pstate) need to modify state on a core before it
is completely offline.  The ->exit() callback is executed during the
CPU_POST_DEAD phase of the cpu offline process which is too late to
change the state of the core.

Patch 1 adds an optional callback function to the cpufreq_driver
interface which is called by the core during the CPU_DOWN_PREPARE
phase of cpu offline in __cpufreq_remove_dev_prepare().

Patch 2 changes intel_pstate to use the ->stop callback to do
its cleanup during cpu offline. 

Dirk Brandewie (2):
  cpufreq: Add exit_prepare callback to cpufreq_driver interface
  intel_pstate: Set core to min P state during core offline

 Documentation/cpu-freq/cpu-drivers.txt |  8 +++++++-
 drivers/cpufreq/cpufreq.c              |  3 +++
 drivers/cpufreq/intel_pstate.c         | 20 +++++++++++++-------
 include/linux/cpufreq.h                |  1 +
 4 files changed, 24 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] cpufreq: Add stop callback to cpufreq_driver interface

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

This callback allows the driver to do clean up before the CPU is
completely down and its state cannot be modified.  This is used
by the intel_pstate driver to reduce the requested P state prior to
the core going away.  This is required because the requested P state
of the offline core is used to select the package P state. This
effectively sets the floor package P state to the requested P state on
the offline core.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 Documentation/cpu-freq/cpu-drivers.txt | 8 +++++++-
 drivers/cpufreq/cpufreq.c              | 3 ++-
 include/linux/cpufreq.h                | 1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 8b1a445..79def80 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -61,7 +61,13 @@ target_index		-	See below on the differences.
 
 And optionally
 
-cpufreq_driver.exit -		A pointer to a per-CPU cleanup function.
+cpufreq_driver.exit -		A pointer to a per-CPU cleanup
+		    		function called during CPU_POST_DEAD
+		    		phase of cpu hotplug process.
+
+cpufreq_driver.stop -		A pointer to a per-CPU stop function
+			    	called during CPU_DOWN_PREPARE phase of
+				cpu hotplug process.
 
 cpufreq_driver.resume -		A pointer to a per-CPU resume function
 				which is called with interrupts disabled
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index cf485d9..bb20292 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1336,7 +1336,8 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
 						__func__, new_cpu, cpu);
 			}
 		}
-	}
+	} else if (cpufreq_driver->stop && cpufreq_driver->setpolicy)
+		cpufreq_driver->stop(policy);
 
 	return 0;
 }
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 4d89e0e..ff8db19 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -224,6 +224,7 @@ struct cpufreq_driver {
 	int	(*bios_limit)	(int cpu, unsigned int *limit);
 
 	int	(*exit)		(struct cpufreq_policy *policy);
+	int	(*stop)		(struct cpufreq_policy *policy);
 	int	(*suspend)	(struct cpufreq_policy *policy);
 	int	(*resume)	(struct cpufreq_policy *policy);
 	struct freq_attr	**attr;
-- 
1.8.3.1

[PATCH 2/2] intel_pstate: Set core to min P state during core offline

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Change to use ->exit_prepare() callback to do clean up during CPU
hotplug. The requested P state for an offline core will be used by the
hardware coordination function to select the package P state. If the
core is under load when it is offlined it will fix the package P state
floor to the requested P state of offline core.

Reported-by: Patrick Marlier <patrick.marlier@gmail.com>
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2cd36b9..e9092fd 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -447,7 +447,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
 	if (limits.no_turbo)
 		val |= (u64)1 << 32;
 
-	wrmsrl(MSR_IA32_PERF_CTL, val);
+	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
 }
 
 static struct cpu_defaults core_params = {
@@ -773,16 +773,22 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
+static int intel_pstate_cpu_stop(struct cpufreq_policy *policy)
 {
-	int cpu = policy->cpu;
+	int cpu_num = policy->cpu;
+	struct cpudata *cpu = all_cpu_data[cpu_num];
+
+	pr_info("intel_pstate CPU %d exiting\n", cpu_num);
+
+	del_timer(&all_cpu_data[cpu_num]->timer);
+	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
+	kfree(all_cpu_data[cpu_num]);
+	all_cpu_data[cpu_num] = NULL;
 
-	del_timer(&all_cpu_data[cpu]->timer);
-	kfree(all_cpu_data[cpu]);
-	all_cpu_data[cpu] = NULL;
 	return 0;
 }
 
+
 static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 {
 	struct cpudata *cpu;
@@ -818,7 +824,7 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.setpolicy	= intel_pstate_set_policy,
 	.get		= intel_pstate_get,
 	.init		= intel_pstate_cpu_init,
-	.exit		= intel_pstate_cpu_exit,
+	.stop		= intel_pstate_cpu_stop,
 	.name		= "intel_pstate",
 };
 
-- 
1.8.3.1

Re: [PATCH 1/2] cpufreq: Add stop callback to cpufreq_driver interface

[Rafael J. Wysocki]

On Wednesday, March 19, 2014 08:45:53 AM dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> This callback allows the driver to do clean up before the CPU is
> completely down and its state cannot be modified.  This is used
> by the intel_pstate driver to reduce the requested P state prior to
> the core going away.  This is required because the requested P state
> of the offline core is used to select the package P state. This
> effectively sets the floor package P state to the requested P state on
> the offline core.
> 
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  Documentation/cpu-freq/cpu-drivers.txt | 8 +++++++-
>  drivers/cpufreq/cpufreq.c              | 3 ++-
>  include/linux/cpufreq.h                | 1 +
>  3 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 8b1a445..79def80 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -61,7 +61,13 @@ target_index		-	See below on the differences.
>  
>  And optionally
>  
> -cpufreq_driver.exit -		A pointer to a per-CPU cleanup function.
> +cpufreq_driver.exit -		A pointer to a per-CPU cleanup
> +		    		function called during CPU_POST_DEAD
> +		    		phase of cpu hotplug process.
> +
> +cpufreq_driver.stop -		A pointer to a per-CPU stop function
> +			    	called during CPU_DOWN_PREPARE phase of
> +				cpu hotplug process.
>  
>  cpufreq_driver.resume -		A pointer to a per-CPU resume function
>  				which is called with interrupts disabled
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index cf485d9..bb20292 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1336,7 +1336,8 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
>  						__func__, new_cpu, cpu);
>  			}
>  		}
> -	}
> +	} else if (cpufreq_driver->stop && cpufreq_driver->setpolicy)
> +		cpufreq_driver->stop(policy);

Nit: Why is it an int function if the only caller doesn't check the
return value?  It should be void.

>  
>  	return 0;
>  }
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 4d89e0e..ff8db19 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -224,6 +224,7 @@ struct cpufreq_driver {
>  	int	(*bios_limit)	(int cpu, unsigned int *limit);
>  
>  	int	(*exit)		(struct cpufreq_policy *policy);
> +	int	(*stop)		(struct cpufreq_policy *policy);
>  	int	(*suspend)	(struct cpufreq_policy *policy);
>  	int	(*resume)	(struct cpufreq_policy *policy);
>  	struct freq_attr	**attr;
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] intel_pstate: Set core to min P state during core offline

[Viresh Kumar]

On 19 March 2014 21:15,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
>
> Change to use ->exit_prepare() callback to do clean up during CPU

exit_prepare() ?? Probably rename it to cpu_stop as well, as Rafael
suggested in another thread.

> hotplug. The requested P state for an offline core will be used by the
> hardware coordination function to select the package P state. If the
> core is under load when it is offlined it will fix the package P state
> floor to the requested P state of offline core.
>
> Reported-by: Patrick Marlier <patrick.marlier@gmail.com>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2cd36b9..e9092fd 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -447,7 +447,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
>         if (limits.no_turbo)
>                 val |= (u64)1 << 32;
>
> -       wrmsrl(MSR_IA32_PERF_CTL, val);
> +       wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
>  }
>
>  static struct cpu_defaults core_params = {
> @@ -773,16 +773,22 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
>         return 0;
>  }
>
> -static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
> +static int intel_pstate_cpu_stop(struct cpufreq_policy *policy)
>  {
> -       int cpu = policy->cpu;
> +       int cpu_num = policy->cpu;
> +       struct cpudata *cpu = all_cpu_data[cpu_num];
> +
> +       pr_info("intel_pstate CPU %d exiting\n", cpu_num);
> +
> +       del_timer(&all_cpu_data[cpu_num]->timer);
> +       intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
> +       kfree(all_cpu_data[cpu_num]);
> +       all_cpu_data[cpu_num] = NULL;
>
> -       del_timer(&all_cpu_data[cpu]->timer);
> -       kfree(all_cpu_data[cpu]);
> -       all_cpu_data[cpu] = NULL;
>         return 0;
>  }
>
> +
>  static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
>  {
>         struct cpudata *cpu;
> @@ -818,7 +824,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>         .setpolicy      = intel_pstate_set_policy,
>         .get            = intel_pstate_get,
>         .init           = intel_pstate_cpu_init,
> -       .exit           = intel_pstate_cpu_exit,
> +       .stop           = intel_pstate_cpu_stop,
>         .name           = "intel_pstate",
>  };
>
> --
> 1.8.3.1
>

Re: [PATCH 1/2] cpufreq: Add stop callback to cpufreq_driver interface

[Rafael J. Wysocki]

On Wednesday, March 19, 2014 11:31:31 PM Rafael J. Wysocki wrote:
> On Wednesday, March 19, 2014 08:45:53 AM dirk.brandewie@gmail.com wrote:
> > From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > 
> > This callback allows the driver to do clean up before the CPU is
> > completely down and its state cannot be modified.  This is used
> > by the intel_pstate driver to reduce the requested P state prior to
> > the core going away.  This is required because the requested P state
> > of the offline core is used to select the package P state. This
> > effectively sets the floor package P state to the requested P state on
> > the offline core.
> > 
> > Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>

To speed up things a bit I changed the name of the new callback to ->stop_cpu
and made it void.

I also modified patch [2/2] accordingly and queued the both of them up for 3.15.

Please check the bleeding-edge branch.

I wouldn't like to need to do anything like that any more in the future, though.

> > ---
> >  Documentation/cpu-freq/cpu-drivers.txt | 8 +++++++-
> >  drivers/cpufreq/cpufreq.c              | 3 ++-
> >  include/linux/cpufreq.h                | 1 +
> >  3 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> > index 8b1a445..79def80 100644
> > --- a/Documentation/cpu-freq/cpu-drivers.txt
> > +++ b/Documentation/cpu-freq/cpu-drivers.txt
> > @@ -61,7 +61,13 @@ target_index		-	See below on the differences.
> >  
> >  And optionally
> >  
> > -cpufreq_driver.exit -		A pointer to a per-CPU cleanup function.
> > +cpufreq_driver.exit -		A pointer to a per-CPU cleanup
> > +		    		function called during CPU_POST_DEAD
> > +		    		phase of cpu hotplug process.
> > +
> > +cpufreq_driver.stop -		A pointer to a per-CPU stop function
> > +			    	called during CPU_DOWN_PREPARE phase of
> > +				cpu hotplug process.
> >  
> >  cpufreq_driver.resume -		A pointer to a per-CPU resume function
> >  				which is called with interrupts disabled
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index cf485d9..bb20292 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1336,7 +1336,8 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
> >  						__func__, new_cpu, cpu);
> >  			}
> >  		}
> > -	}
> > +	} else if (cpufreq_driver->stop && cpufreq_driver->setpolicy)
> > +		cpufreq_driver->stop(policy);
> 
> Nit: Why is it an int function if the only caller doesn't check the
> return value?  It should be void.
> 
> >  
> >  	return 0;
> >  }
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 4d89e0e..ff8db19 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -224,6 +224,7 @@ struct cpufreq_driver {
> >  	int	(*bios_limit)	(int cpu, unsigned int *limit);
> >  
> >  	int	(*exit)		(struct cpufreq_policy *policy);
> > +	int	(*stop)		(struct cpufreq_policy *policy);
> >  	int	(*suspend)	(struct cpufreq_policy *policy);
> >  	int	(*resume)	(struct cpufreq_policy *policy);
> >  	struct freq_attr	**attr;
> > 
> 
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/2] cpufreq: Add stop callback to cpufreq_driver interface

[Dirk Brandewie]

On 03/19/2014 08:28 PM, Rafael J. Wysocki wrote:
> On Wednesday, March 19, 2014 11:31:31 PM Rafael J. Wysocki wrote:
>> On Wednesday, March 19, 2014 08:45:53 AM dirk.brandewie@gmail.com wrote:
>>> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
>>>
>>> This callback allows the driver to do clean up before the CPU is
>>> completely down and its state cannot be modified.  This is used
>>> by the intel_pstate driver to reduce the requested P state prior to
>>> the core going away.  This is required because the requested P state
>>> of the offline core is used to select the package P state. This
>>> effectively sets the floor package P state to the requested P state on
>>> the offline core.
>>>
>>> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
>
> To speed up things a bit I changed the name of the new callback to ->stop_cpu
> and made it void.
>
> I also modified patch [2/2] accordingly and queued the both of them up for 3.15.
>
> Please check the bleeding-edge branch.
>
> I wouldn't like to need to do anything like that any more in the future, though.

Sorry had appointments yesterday afternoon.

bleeding-edge looks good


>
>>> ---
>>>   Documentation/cpu-freq/cpu-drivers.txt | 8 +++++++-
>>>   drivers/cpufreq/cpufreq.c              | 3 ++-
>>>   include/linux/cpufreq.h                | 1 +
>>>   3 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
>>> index 8b1a445..79def80 100644
>>> --- a/Documentation/cpu-freq/cpu-drivers.txt
>>> +++ b/Documentation/cpu-freq/cpu-drivers.txt
>>> @@ -61,7 +61,13 @@ target_index		-	See below on the differences.
>>>
>>>   And optionally
>>>
>>> -cpufreq_driver.exit -		A pointer to a per-CPU cleanup function.
>>> +cpufreq_driver.exit -		A pointer to a per-CPU cleanup
>>> +		    		function called during CPU_POST_DEAD
>>> +		    		phase of cpu hotplug process.
>>> +
>>> +cpufreq_driver.stop -		A pointer to a per-CPU stop function
>>> +			    	called during CPU_DOWN_PREPARE phase of
>>> +				cpu hotplug process.
>>>
>>>   cpufreq_driver.resume -		A pointer to a per-CPU resume function
>>>   				which is called with interrupts disabled
>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>> index cf485d9..bb20292 100644
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -1336,7 +1336,8 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
>>>   						__func__, new_cpu, cpu);
>>>   			}
>>>   		}
>>> -	}
>>> +	} else if (cpufreq_driver->stop && cpufreq_driver->setpolicy)
>>> +		cpufreq_driver->stop(policy);
>>
>> Nit: Why is it an int function if the only caller doesn't check the
>> return value?  It should be void.
>>
>>>
>>>   	return 0;
>>>   }
>>> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
>>> index 4d89e0e..ff8db19 100644
>>> --- a/include/linux/cpufreq.h
>>> +++ b/include/linux/cpufreq.h
>>> @@ -224,6 +224,7 @@ struct cpufreq_driver {
>>>   	int	(*bios_limit)	(int cpu, unsigned int *limit);
>>>
>>>   	int	(*exit)		(struct cpufreq_policy *policy);
>>> +	int	(*stop)		(struct cpufreq_policy *policy);
>>>   	int	(*suspend)	(struct cpufreq_policy *policy);
>>>   	int	(*resume)	(struct cpufreq_policy *policy);
>>>   	struct freq_attr	**attr;
>>>
>>
>>
>

Re: [PATCH 2/2] intel_pstate: Set core to min P state during core offline

[Srivatsa S. Bhat]

On 03/18/2014 10:52 PM, dirk.brandewie@gmail.com wrote:
> From: Dirk Brandewie <dirk.j.brandewie@intel.com>
> 
> Change to use ->exit_prepare() callback to do clean up during CPU

->stop()

> hotplug. The requested P state for an offline core will be used by the
> hardware coordination function to select the package P state. If the
> core is under load when it is offlined it will fix the package P state
> floor to the requested P state of offline core.
> 
> Reported-by: Patrick Marlier <patrick.marlier@gmail.com>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 2cd36b9..e9092fd 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -447,7 +447,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
>  	if (limits.no_turbo)
>  		val |= (u64)1 << 32;
> 
> -	wrmsrl(MSR_IA32_PERF_CTL, val);
> +	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
>  }
> 
>  static struct cpu_defaults core_params = {
> @@ -773,16 +773,22 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
>  	return 0;
>  }
> 
> -static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
> +static int intel_pstate_cpu_stop(struct cpufreq_policy *policy)
>  {
> -	int cpu = policy->cpu;
> +	int cpu_num = policy->cpu;
> +	struct cpudata *cpu = all_cpu_data[cpu_num];
> +
> +	pr_info("intel_pstate CPU %d exiting\n", cpu_num);
> +
> +	del_timer(&all_cpu_data[cpu_num]->timer);
> +	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
> +	kfree(all_cpu_data[cpu_num]);
> +	all_cpu_data[cpu_num] = NULL;
>

I think it should be relatively simple to keep the intel_pstate_set_pstate()
here inside ->stop() and move the rest of the code to ->exit().

Regards,
Srivatsa S. Bhat
 
> -	del_timer(&all_cpu_data[cpu]->timer);
> -	kfree(all_cpu_data[cpu]);
> -	all_cpu_data[cpu] = NULL;
>  	return 0;
>  }
> 
> +
>  static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
>  {
>  	struct cpudata *cpu;
> @@ -818,7 +824,7 @@ static struct cpufreq_driver intel_pstate_driver = {
>  	.setpolicy	= intel_pstate_set_policy,
>  	.get		= intel_pstate_get,
>  	.init		= intel_pstate_cpu_init,
> -	.exit		= intel_pstate_cpu_exit,
> +	.stop		= intel_pstate_cpu_stop,
>  	.name		= "intel_pstate",
>  };
> 

[PATCH 2/2] intel_pstate: Set core to min P state during core offline

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Change to use ->exit_prepare() callback to do clean up during CPU
hotplug. The requested P state for an offline core will be used by the
hardware coordination function to select the package P state. If the
core is under load when it is offlined it will fix the package P state
floor to the requested P state of offline core.

Reported-by: Patrick Marlier <patrick.marlier@gmail.com>
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2cd36b9..e9092fd 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -447,7 +447,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
 	if (limits.no_turbo)
 		val |= (u64)1 << 32;
 
-	wrmsrl(MSR_IA32_PERF_CTL, val);
+	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
 }
 
 static struct cpu_defaults core_params = {
@@ -773,16 +773,22 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
+static int intel_pstate_cpu_stop(struct cpufreq_policy *policy)
 {
-	int cpu = policy->cpu;
+	int cpu_num = policy->cpu;
+	struct cpudata *cpu = all_cpu_data[cpu_num];
+
+	pr_info("intel_pstate CPU %d exiting\n", cpu_num);
+
+	del_timer(&all_cpu_data[cpu_num]->timer);
+	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
+	kfree(all_cpu_data[cpu_num]);
+	all_cpu_data[cpu_num] = NULL;
 
-	del_timer(&all_cpu_data[cpu]->timer);
-	kfree(all_cpu_data[cpu]);
-	all_cpu_data[cpu] = NULL;
 	return 0;
 }
 
+
 static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 {
 	struct cpudata *cpu;
@@ -818,7 +824,7 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.setpolicy	= intel_pstate_set_policy,
 	.get		= intel_pstate_get,
 	.init		= intel_pstate_cpu_init,
-	.exit		= intel_pstate_cpu_exit,
+	.stop		= intel_pstate_cpu_stop,
 	.name		= "intel_pstate",
 };
 
-- 
1.8.3.1

[PATCH 2/2] intel_pstate: Set core to min P state during core offline

[dirk.brandewie]

From: Dirk Brandewie <dirk.j.brandewie@intel.com>

Change to use ->exit_prepare() callback to do clean up during CPU
hotplug. The requested P state for an offline core will be used by the
hardware coordination function to select the package P state. If the
core is under load when it is offlined it will fix the package P state
floor to the requested P state of offline core.

Reported-by: Patrick Marlier <patrick.marlier@gmail.com>
Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 2cd36b9..fe32a8c 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -447,7 +447,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
 	if (limits.no_turbo)
 		val |= (u64)1 << 32;
 
-	wrmsrl(MSR_IA32_PERF_CTL, val);
+	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
 }
 
 static struct cpu_defaults core_params = {
@@ -773,16 +773,22 @@ static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
+static int intel_pstate_cpu_exit_prepare(struct cpufreq_policy *policy)
 {
-	int cpu = policy->cpu;
+	int cpu_num = policy->cpu;
+	struct cpudata *cpu = all_cpu_data[cpu_num];
+
+	pr_info("intel_pstate CPU %d exiting\n", cpu_num);
+
+	del_timer(&all_cpu_data[cpu_num]->timer);	
+	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
+	kfree(all_cpu_data[cpu_num]);
+	all_cpu_data[cpu_num] = NULL;
 
-	del_timer(&all_cpu_data[cpu]->timer);
-	kfree(all_cpu_data[cpu]);
-	all_cpu_data[cpu] = NULL;
 	return 0;
 }
 
+
 static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 {
 	struct cpudata *cpu;
@@ -818,7 +824,7 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.setpolicy	= intel_pstate_set_policy,
 	.get		= intel_pstate_get,
 	.init		= intel_pstate_cpu_init,
-	.exit		= intel_pstate_cpu_exit,
+	.exit_prepare	= intel_pstate_cpu_exit_prepare,
 	.name		= "intel_pstate",
 };
 
-- 
1.8.3.1