Re: [PATCH] cpufreq, store_scaling_governor requires policy->rwsem to be held for duration of changing governors [v2]

[Prarit Bhargava]

On 08/12/2014 05:03 AM, Viresh Kumar wrote:
> On 7 August 2014 15:45, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>> On 7 August 2014 15:42, Prarit Bhargava <prarit@xxxxxxxxxx> wrote:
>>> That should have done it.  What are your CPUFREQ configs?
>>
>> You can check the same .config I attached last time for that :)
>>
>> CONFIG_CPU_FREQ=y
>> CONFIG_CPU_FREQ_GOV_COMMON=y
>> CONFIG_CPU_FREQ_STAT=y
>> CONFIG_CPU_FREQ_STAT_DETAILS=y
>> # CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE is not set
>> # CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE is not set
>> # CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND is not set
>> CONFIG_CPU_FREQ_DEFAULT_GOV_CONSERVATIVE=y
>> CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
>> CONFIG_CPU_FREQ_GOV_POWERSAVE=y
>> CONFIG_CPU_FREQ_GOV_USERSPACE=y
>> CONFIG_CPU_FREQ_GOV_ONDEMAND=y
>> CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y

Viresh, sorry for my late reply -- I got distracted by another issue.

>>
>>
>> Anyway, has anybody tried to test what I have been trying now?
>> @Prarit: You can try that on your x86 box as well, which has a
>> single cluster or group of CPUs sharing clock line.
> 

Okay, this is what I have and I can reproduce this *easily* 100% of the time.

I've used your above config options and have enabled LOCKDEP.

In order to restore the locking, I've applied the following patch to the cpufreq
core (sorry for the cut-and-paste):

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d9fdedd..dfda238 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2192,9 +2192,7 @@ static int cpufreq_set_policy(struct cpufreq_policy *polic
        /* end old governor */
        if (old_gov) {
                __cpufreq_governor(policy, CPUFREQ_GOV_STOP);
-               up_write(&policy->rwsem);
                __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
-               down_write(&policy->rwsem);
        }

        /* start new governor */
@@ -2203,9 +2201,7 @@ static int cpufreq_set_policy(struct cpufreq_policy *polic
                if (!__cpufreq_governor(policy, CPUFREQ_GOV_START))
                        goto out;

-               up_write(&policy->rwsem);
                __cpufreq_governor(policy, CPUFREQ_GOV_POLICY_EXIT);
-               down_write(&policy->rwsem);
        }

        /* new governor failed, so re-start old one */


I've modified the acpi-cpufreq driver to include (sorry for the cut-and-paste)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index b0c18ed..97653c3 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -884,6 +884,9 @@ static struct freq_attr *acpi_cpufreq_attr[] = {
 };

 static struct cpufreq_driver acpi_cpufreq_driver = {
+       .flags                  = CPUFREQ_STICKY |
+                                       CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
+                                       CPUFREQ_NEED_INITIAL_FREQ_CHECK,
        .verify         = cpufreq_generic_frequency_table_verify,
        .target_index   = acpi_cpufreq_target,
        .bios_limit     = acpi_processor_get_bios_limit,

I do a

cat /sys/devices/system/cpu/cpu2/cpufreq/conservative/*
echo ondemand > /sys/devices/system/cpu/cpu2/cpufreq/scaling_governor

and then I immediately see the stack trace.

P.


> Ping!!
> 
--
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/