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

["Wang, Wei W" <wei.w.wang@xxxxxxxxx>]

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.

Best,
Wei  

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