Re: [Xen-devel] [PATCH v3 07/11] x86/intel_pstate: changes in cpufreq_del_cpu for CPU offline

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.15 at 10:21, <wei.w.wang@xxxxxxxxx> wrote:
> On 23/06/2015 16:08, Jan Beulich wrote:
>> >>> On 23.06.15 at 08:16, <wei.w.wang@xxxxxxxxx> wrote:
>> > On 19/06/2015 17:44, Jan Beulich wrote:
>> >> >>> On 11.06.15 at 10:28, <wei.w.wang@xxxxxxxxx> wrote:
>> >> > cpufreq_cpu_policy is used in intel_pstate_set_pstate(), so we
>> >> > change to NULL it after the call of cpufreq_driver->exit.
>> >> > Otherwise, a calltrace will show up on your screen due to the
>> >> > reference of a NULL pointer when you power down the system.
>> >>
>> >> Apart from what was already said on this patch, I think it is placed
>> >> too late in this series (should precede patch 6) or, even better, not
>> >> needed at all: ->exit() gets passed the policy being cleaned up, so I
>> >> don't follow why the driver needs to consult the per-CPU field to obtain 
>> >> it.
>> >
>> >> Plus, if the patch is to be kept, the description suggesting this to
>> >> be a problem at system shutdown only is wrong - it can equally well
>> >> happen while offlining a CPU. Just saying that the pointer is still needed
>> would be sufficient.
>> >
>> > It's needed. When unplugging a CPU, the intel_pstate driver sets it to
>> > run with the lowest P-state.
>> 
>> I understand the latter, but this doesn't explain why you can't do what I
>> suggested above.
> 
> Because the "->exit()" needs to call "intel_pstate_set_pstate()" which does 
> not receive "policy" as a parameter. "intel_pstate_set_pstate()" is also 
> called by another function without passing "policy". So I think it is simpler 
> to just change the order of NULLing the pointer, instead of changing more 
> code.

I yet have to see the amount of differences to the Linux original to be
convinced that this is the better approach, so let's see how the next
version of the series is going to look like.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel