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

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

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?

Best,
Wei


> > +    if (!driver_data || !driver_data->init
> > +        || !driver_data->verify || !driver_data->exit
> > +        || (!driver_data->target == !driver_data->setpolicy))
> > +        return -EINVAL;
> > +
> > +    if (cpufreq_driver)
> > +        return -EBUSY;
> > +
> > +    cpufreq_driver = driver_data;
> > +
> >      register_cpu_notifier(&cpu_nfb);
> > +
> >      return 0;
> >  }
> > -presmp_initcall(cpufreq_presmp_init);
> >
> > +int cpufreq_unregister_driver(struct cpufreq_driver *driver) {
> > +    if (!cpufreq_driver || (driver != cpufreq_driver))
> > +        return -EINVAL;
> > +
> > +    cpufreq_driver = NULL;
> > +
> > +    unregister_cpu_notifier(&cpu_nfb);
> > +
> > +    return 0;
> > +}
> 
> This function is dead (and perhaps just like unregister_cpu_notifier() not
> even needed without loadable modules).
> 
> Jan


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