Messages in this thread | | | Subject | RE: CPUFREQ-CPUHOTPLUG: Possible circular locking dependency | Date | Wed, 6 Dec 2006 10:27:01 -0800 | From | "Pallipadi, Venkatesh" <> |
| |
>-----Original Message----- >From: linux-kernel-owner@vger.kernel.org >[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of >Gautham R Shenoy >Sent: Thursday, November 30, 2006 3:44 AM >To: Ingo Molnar >Cc: Gautham R Shenoy; akpm@osdl.org; >linux-kernel@vger.kernel.org; torvalds@osdl.org; >davej@redhat.com; dipankar@in.ibm.com; vatsa@in.ibm.com >Subject: Re: CPUFREQ-CPUHOTPLUG: Possible circular locking dependency > >On Thu, Nov 30, 2006 at 12:03:15PM +0100, Ingo Molnar wrote: >> >> * Gautham R Shenoy <ego@in.ibm.com> wrote: >> >> > a) cpufreq maintain's it's own cpumask in the variable >> > policy->affected_cpus and says : If a frequency change is issued to >> > any one of the cpu's in the affected_cpus mask, you change >frequency >> > on all cpus in the mask. So this needs to be consistent with >> > cpu_online map and hence cpu hotplug aware. Furthermore, >we don't want >> > cpus in this mask to go down when we are trying to change >frequencies >> > on them. The function which drives the frequency change in >> > cpufreq-core is cpufreq_driver_target and it needs cpu-hotplug >> > protection. >> >> couldnt this complexity be radically simplified by having new kernel >> infrastructure that does something like: >> >> " 'gather' all CPUs mentioned in <mask> via scheduling a separate >> helper-kthread on every CPU that <mask> specifies, disable all >> interrupts, and execute function <fn> once all CPUs have been >> 'gathered' - and release all CPUs once <fn> has executed >on each of >> them." >> >> ? > >This is what is currently being done by cpufreq: > >a) get_some_cpu_hotplug_protection() [use either some global mechanism > or a persubsystem mutex] > >b) actual_freq_change_driver_function(mask) >/* You can check out cpufreq_p4_target() in > * arch/i386/kernel/cpu/cpufreq/p4-clockmod.c > */ > > { > for_each_cpu_mask(i, mask) { > cpumask_t this_cpu = cpumask_of_cpu(i); > set_cpus_allowed(current, this_cpu); > function_to_change_frequency(); > > } > } >
As there are many options being discussed here, let me propose one more option that can eliminate the need for hotplug lock in cpufreq_driver_target() path.
As Gautham clearly explained before, today we have cpufreq calling cpufreq_driver_target() in each driver to change the CPU frequency And the driver internally uses set_cpus_allowed to reschedule onto different affected_cpus and change the frequency. That is the main reason why we need to disable hotplug in this path.
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.
This means a lot of changes as we need new interface changes to cpufreq and rewrite of bunch of drivers. But, this looks to me as the least complicated solution.
Thanks, Venki - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |