RE: CPUFREQ-CPUHOTPLUG: Possible circular locking dependency

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: linux-kernel-owner@xxxxxxxxxxxxxxx 
>[mailto:linux-kernel-owner@xxxxxxxxxxxxxxx] On Behalf Of 
>Gautham R Shenoy
>Sent: Thursday, November 30, 2006 3:44 AM
>To: Ingo Molnar
>Cc: Gautham R Shenoy; akpm@xxxxxxxx; 
>linux-kernel@xxxxxxxxxxxxxxx; torvalds@xxxxxxxx; 
>davej@xxxxxxxxxx; dipankar@xxxxxxxxxx; vatsa@xxxxxxxxxx
>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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/