>-----Original Message----- >From: [email protected] >[mailto:[email protected]] On Behalf Of >Gautham R Shenoy >Sent: Wednesday, December 06, 2006 11:07 PM >To: Pallipadi, Venkatesh >Cc: [email protected]; Ingo Molnar; [email protected]; >[email protected]; [email protected]; >[email protected]; [email protected]; [email protected] >Subject: Re: CPUFREQ-CPUHOTPLUG: Possible circular locking dependency > >Hi Venki, >On Wed, Dec 06, 2006 at 10:27:01AM -0800, Pallipadi, Venkatesh wrote: > >> But, if we make cpufreq more affected_cpus aware and have a per_cpu >> target() >> call by moving set_cpus_allowed() from driver into cpufreq core and >> define >> the target function to be atomic/non-sleeping type, then we >really don't >> need a hotplug lock for the driver any more. Driver can have >> get_cpu/put_cpu >> pair to disable preemption and then change the frequency. > >Well, we would still need to keep the affected_cpus map in >sync with the >cpu_online map. That would still require hotplug protection, right? Not really. Cpufreq can look at all the CPUs in affected_cpus and call new_target() only for CPUs that are online. Or it can call for every CPU and the actual implementation in driver can do something like set_cpus_allowed(requested_processor_mask) If (get_cpu() != requested_cpu) { /* CPU offline and nothing can be done */ put_cpu(); return 0; } This is what I did for new cpufreq interface I added for getavg(). It was easy to ensure the atomic driver call as only one driver is using it today :-) >Besides, I would love to see a way of implementing target >function to be >atomic/non-sleeping type. But as of now, the target functions call >cpufreq_notify_transition which might sleep. > That is the reason I don't have a patch for this now.. :-) There are more than 10 or so drivers that need to change for new interface. I haven't checked whether it is possible to do this without sleeping in all those drivers. >That's not the last of my worries. The ondemand-workqueue interaction >in the cpu_hotplug callback path can cause a deadlock if we go for >per-subsystem hotcpu mutexes. Can you think of a way by which we can >avoid destroying the kondemand workqueue from the cpu-hotplug callback >path ? Please see http://lkml.org/lkml/2006/11/30/9 for the >culprit-callpath. > Yes. I was thinking about it. One possible solution is to move create_workqueue()/destroy_workqueue() as in attached patch. Thanks, Venki Not fully tested at the moment. Remove callbacks using workqueue callbacks in governor start and stop. And move those call back to module init and exit time. Signed-off-by: Venkatesh Pallipadi <[email protected]> Index: linux-2.6.19-rc-mm/drivers/cpufreq/cpufreq_ondemand.c =================================================================== --- linux-2.6.19-rc-mm.orig/drivers/cpufreq/cpufreq_ondemand.c +++ linux-2.6.19-rc-mm/drivers/cpufreq/cpufreq_ondemand.c @@ -514,7 +514,6 @@ static inline void dbs_timer_exit(struct { dbs_info->enable = 0; cancel_delayed_work(&dbs_info->work); - flush_workqueue(kondemand_wq); } static int cpufreq_governor_dbs(struct cpufreq_policy *policy, @@ -543,16 +542,6 @@ static int cpufreq_governor_dbs(struct c mutex_lock(&dbs_mutex); dbs_enable++; - if (dbs_enable == 1) { - kondemand_wq = create_workqueue("kondemand"); - if (!kondemand_wq) { - printk(KERN_ERR - "Creation of kondemand failed\n"); - dbs_enable--; - mutex_unlock(&dbs_mutex); - return -ENOSPC; - } - } rc = sysfs_create_group(&policy->kobj, &dbs_attr_group); if (rc) { @@ -601,8 +590,6 @@ static int cpufreq_governor_dbs(struct c dbs_timer_exit(this_dbs_info); sysfs_remove_group(&policy->kobj, &dbs_attr_group); dbs_enable--; - if (dbs_enable == 0) - destroy_workqueue(kondemand_wq); mutex_unlock(&dbs_mutex); @@ -632,12 +619,18 @@ static struct cpufreq_governor cpufreq_g static int __init cpufreq_gov_dbs_init(void) { + kondemand_wq = create_workqueue("kondemand"); + if (!kondemand_wq) { + printk(KERN_ERR "Creation of kondemand failed\n"); + return -EFAULT; + } return cpufreq_register_governor(&cpufreq_gov_dbs); } static void __exit cpufreq_gov_dbs_exit(void) { cpufreq_unregister_governor(&cpufreq_gov_dbs); + destroy_workqueue(kondemand_wq); }
Attachment:
ondemand_recursive_locking_fix.patch
Description: ondemand_recursive_locking_fix.patch
- Prev by Date: [PATCH] WorkStruct: Fix spi_bitbang.h
- Next by Date: Re: What happen when hangs !!
- Previous by thread: Re: CPUFREQ-CPUHOTPLUG: Possible circular locking dependency
- Next by thread: [PATCH] i386 add idle notifier
- Index(es):
 |