[PATCH] cpuidle: fix unremovable issue for module driver

[Rafael J. Wysocki]

On Tuesday, July 30, 2013 03:33:44 PM Rafael J. Wysocki wrote:
> On Tuesday, July 30, 2013 01:19:46 PM Daniel Lezcano wrote:
> > On 07/30/2013 12:48 PM, Wang Dongsheng-B40534 wrote:
> > > 
> > > 
> > >> -----Original Message-----
> > >> From: Daniel Lezcano [mailto:daniel.lezcano at linaro.org]
> > >> Sent: Tuesday, July 30, 2013 5:28 PM
> > >> To: Wang Dongsheng-B40534
> > >> Cc: rjw at sisk.pl; linux-pm at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
> > >> Subject: Re: [PATCH] cpuidle: fix unremovable issue for module driver
> > >>
> > >> On 07/30/2013 08:55 AM, Dongsheng Wang wrote:
> > >>> From: Wang Dongsheng <dongsheng.wang at freescale.com>
> > >>>
> > >>> After __cpuidle_register_device, the cpu incs are added up, but decs
> > >>> are not, thus the module refcount is not match. So the module "exit"
> > >>> function can not be executed when we do remove operation. Move
> > >>> module_put into __cpuidle_register_device to fix it.
> > >>
> > >> Sorry, I still don't get it :/
> > >>
> > >> register->module_get
> > >> unregister->module_put
> > >>
> > >> you change it by:
> > >>
> > >> register->module_get
> > >> register->module_put
> > >> unregister->none
> > >>
> > >> which is wrong.
> > >>
> > > module_get->set per cpu incs=1
> > > module_put->set per cpu decs=1
> > > 
> > > module_refcount-> incs - decs;
> > > 
> > > "unregister" usually call in module->exit function. 
> > > But if module_refcount is not zero, the module->exit() cannot be executed.
> > 
> > Ok, IIUC, the refcount is decremented in the unregister function but
> > this one is never called because the refcount is not zero.
> > 
> > Funny, that means the module format was *never* used at all as it does
> > not work.
> > 
> > I am wondering if we shouldn't just remove the module support for
> > cpuidle. Rafael ?
> 
> That would be the simplest thing to do and possibly the most correct one too,
> but I need to double check how inte_idle/ACPI idle interactions depend on that.

As I thought, the ordering between intel_idle and the ACPI processor driver's
idle part depends on the latter being modular.

Also I'm not sure if the core is at fault here.  It evidently expects that
drivers will be registered before devices, so the driver module cannot be
released as long as there are any active devices.  This sounds logical from
the correctness viewpoint.

However, from the usability viewpoint it is not very convenient.  It would be
nicer if the driver could be unregistered and registered while devices are
there, but I'm afraid that would require some code reorganization.

The $subject patch isn't a correct change anyway in my opinion, because it
tries to kind of sidestep the core's assumptions.

Thanks,
Rafael


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