[PATCH] cpuidle: fix unremovable issue for module driver

Wang Dongsheng-B40534 B40534 at freescale.com
Tue Jul 30 20:48:18 EST 2013



> -----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.

See, kernel/module.c +874
delete_module()-->try_stop_module();

Test Log:
root:~# rmmod cpuidle-e500
module_refcount: incs[9], decs[1]
rmmod: can't unload 'cpuidle_e500': Resource temporarily unavailable

> Can you describe the problem you are facing ? (a bit more than "I can't
> unload the module").
> 
> > Signed-off-by: Wang Dongsheng <dongsheng.wang at freescale.com>
> >
> > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> > index d75040d..e964ada 100644
> > --- a/drivers/cpuidle/cpuidle.c
> > +++ b/drivers/cpuidle/cpuidle.c
> > @@ -351,11 +351,8 @@ EXPORT_SYMBOL_GPL(cpuidle_disable_device);
> >
> >  static void __cpuidle_unregister_device(struct cpuidle_device *dev)
> > {
> > -	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> > -
> >  	list_del(&dev->device_list);
> >  	per_cpu(cpuidle_devices, dev->cpu) = NULL;
> > -	module_put(drv->owner);
> >  }
> >
> >  static int __cpuidle_device_init(struct cpuidle_device *dev) @@
> > -384,6 +381,8 @@ static int __cpuidle_register_device(struct
> cpuidle_device *dev)
> >  	per_cpu(cpuidle_devices, dev->cpu) = dev;
> >  	list_add(&dev->device_list, &cpuidle_detected_devices);
> >
> > +	module_put(drv->owner);
> > +
> >  	ret = cpuidle_coupled_register_device(dev);
> >  	if (ret) {
> >  		__cpuidle_unregister_device(dev);
> >
> 
> 
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
> 



More information about the Linuxppc-dev mailing list