[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 5/9] x86/intel_pstate: relocate the driver register/unregister function



>>> On 29.05.15 at 04:47, <wei.w.wang@xxxxxxxxx> wrote:
> On 26/05/2015 21:06, Jan Beulich wrote
>> >>> On 13.05.16 at 09:50, <wei.w.wang@xxxxxxxxx> wrote:
>> > Register/unregister the CPU hotplug notifier when the driver is
>> > registered, and move the driver register/unregister function to the
>> > cpufreq.c.
>> 
>> Without saying why I'm afraid I don't even see much reason to review this in
>> any detail.
>> 
>> > --- a/xen/drivers/cpufreq/cpufreq.c
>> > +++ b/xen/drivers/cpufreq/cpufreq.c
>> > @@ -630,12 +630,31 @@ static struct notifier_block cpu_nfb = {
>> >      .notifier_call = cpu_callback
>> >  };
>> >
>> > -static int __init cpufreq_presmp_init(void)
>> > +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
>> >  {
>> > -    void *cpu = (void *)(long)smp_processor_id();
>> > -    cpu_callback(&cpu_nfb, CPU_ONLINE, cpu);
>> 
>> Why is this being removed without replacement?
> 
> I think they are redundant here. 
> If we go and check the hypercall code path (the bottom of set_px_pminfo()),  
> the cpufreq_add_cpu() is called there (inside cpufreq_cpu_init()), too. Why 
> do we need to initialize this CPU twice?

If this is indeed being done twice, removing it is fine. But in a
separate patch with a proper description.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.