2013/8/14 Viresh Kumar <viresh.kumar@xxxxxxxxxx>: > On 13 August 2013 12:39, Xiaoguang Chen <chenxg@xxxxxxxxxxx> wrote: >> __cpufreq_governor operation needs to be executed one by one. >> If one operation is ongoing, the other operation can't be executed. >> If the order is not guaranteed, there may be unexpected behavior. > > What order?? I mean one stop operation is ongoing, one other process tries to call a start operation. > >> For example, governor is in enable state, and one process >> tries to stop the goveror, but it is scheduled out before policy-> >> governor->governor() is executed, but the governor enable flag is >> set to false already. Then one other process tries to start governor, >> It finds enable flag is false, and it can process down to do governor >> start operation, So the governor is started twice. > > That's not possible. A process will not and should not call START > before calling STOP. And so the order of calling these routines must > be forced. > > Hence, we may not need your patch. Please see below code in __cpufreq_governor function mutex_lock(&cpufreq_governor_lock); if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) || //////////// <1> Here one process A tries to stop governor, it finds governor is enabled, so it will pass down. (policy->governor_enabled && (event == CPUFREQ_GOV_START))) { /////////////<3> Process B tries to start governor, it finds enable flag is false, so it can also pass down. mutex_unlock(&cpufreq_governor_lock); return -EBUSY; } if (event == CPUFREQ_GOV_STOP) policy->governor_enabled = false; //////////// < 2> Here process A set flag to false and then process A is scheduled out for some reasons(like interrupt or time slice end) else if (event == CPUFREQ_GOV_START) policy->governor_enabled = true; mutex_unlock(&cpufreq_governor_lock); ret = policy->governor->governor(policy, event); ///////////////<4> Process B executes the governor start operation, process A is not scheduled back yet. as policy->governor->governor is not protected by the cpufreq_governor_lock, So this sequence can happen really. Thanks Xiaoguang -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html