On 03/20/2014 10:09 AM, Viresh Kumar wrote:
> On 19 March 2014 17:45, Srivatsa S. Bhat
> <srivatsa.bhat@xxxxxxxxxxxxxxxxxx> wrote:
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> index 199b52b..e90388f 100644
>> --- a/drivers/cpufreq/cpufreq.c
>> +++ b/drivers/cpufreq/cpufreq.c
>> @@ -349,6 +349,38 @@ void cpufreq_notify_post_transition(struct cpufreq_policy *policy,
>> EXPORT_SYMBOL_GPL(cpufreq_notify_post_transition);
>>
>>
>> +void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
>> + struct cpufreq_freqs *freqs, unsigned int state)
>> +{
>> +wait:
>> + wait_event(&policy->transition_wait, !policy->transition_ongoing);
>
> I think its broken here. At this point another thread can come take lock,
> update transition_ongoing, send notification and finally unlock..
>
> And after that we can take the lock and send another notification..
>
> Correct?
>
Good catch! I missed that yesterday. Please find the updated patch below,
with all your suggestions incorporated. Does this version look any better?
------------------------------------------------------------------------
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 199b52b..5283f10 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -349,6 +349,39 @@ void cpufreq_notify_post_transition(struct cpufreq_policy *policy,
EXPORT_SYMBOL_GPL(cpufreq_notify_post_transition);
+void cpufreq_freq_transition_begin(struct cpufreq_policy *policy,
+ struct cpufreq_freqs *freqs, unsigned int state)
+{
+wait:
+ wait_event(&policy->transition_wait, !policy->transition_ongoing);
+
+ mutex_lock(&policy->transition_lock);
+
+ if (policy->transition_ongoing) {
+ mutex_unlock(&policy->transition_lock);
+ goto wait;
+ }
+
+ policy->transition_ongoing = true;
+
+ mutex_unlock(&policy->transition_lock);
+
+ cpufreq_notify_transition(policy, freqs, CPUFREQ_PRECHANGE);
+}
+
+void cpufreq_freq_transition_end(struct cpufreq_policy *policy,
+ struct cpufreq_freqs *freqs, unsigned int state)
+{
+ cpufreq_notify_transition(policy, freqs, CPUFREQ_POSTCHANGE);
+
+ mutex_lock(&policy->transition_lock);
+ policy->transition_ongoing = false;
+ mutex_unlock(&policy->transition_lock);
+
+ wake_up(&policy->transition_wait);
+}
+
+
/*********************************************************************
* SYSFS INTERFACE *
*********************************************************************/
@@ -968,6 +1001,8 @@ static struct cpufreq_policy *cpufreq_policy_alloc(void)
INIT_LIST_HEAD(&policy->policy_list);
init_rwsem(&policy->rwsem);
+ mutex_init(&policy->transition_lock);
+ init_waitqueue_head(&policy->transition_wait);
return policy;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 4d89e0e..8bded24 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -101,6 +101,11 @@ struct cpufreq_policy {
* __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
*/
struct rw_semaphore rwsem;
+
+ /* Synchronization for frequency transitions */
+ bool transition_ongoing; /* Tracks transition status */
+ struct mutex transition_lock;
+ wait_queue_head_t transition_wait;
};
/* Only for ACPI */
--
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
