| Message ID | CAKohponyMG-uA93ZpH-KD+EOQPmk5tT_QoaQ7+NCEf48vYj5-w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Monday, June 24, 2013 07:01:59 PM Viresh Kumar wrote: > On 24 June 2013 19:03, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > Looks OK, but since transition_ongoing is either 0 or 1 now, as far as I can > > say, it would be better to make it a bool and use = true/false instead of > > ++/-- I suppose. > > Another fixup: > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 6ca7eac..49d942a 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -108,7 +108,7 @@ static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list); > static struct srcu_notifier_head cpufreq_transition_notifier_list; > > /* Tracks status of transition */ > -static int transition_ongoing; > +static bool transition_ongoing; > > static bool init_cpufreq_transition_notifier_list_called; > static int __init init_cpufreq_transition_notifier_list(void) > @@ -271,7 +271,7 @@ void __cpufreq_notify_transition(struct > cpufreq_policy *policy, > "In middle of another frequency transition\n")) > return; > > - transition_ongoing++; > + transition_ongoing = true; > > /* detect if the driver reported a value as "old frequency" > * which is not equal to what the cpufreq core thinks is > @@ -296,7 +296,7 @@ void __cpufreq_notify_transition(struct > cpufreq_policy *policy, > "No frequency transition in progress\n")) > return; > > - transition_ongoing--; > + transition_ongoing = false; > > adjust_jiffies(CPUFREQ_POSTCHANGE, freqs); > pr_debug("FREQ: %lu - CPU: %lu", (unsigned long)freqs->new, Well, now, seeing that the locking around this seems to be kind of haphazard, I'm wondering what prevents two different threads from doing CPUFREQ_PRECHANGE concurrently in such a way that thread A will check transition_ongoing and thread B will check transition_ongoing and then both will set it if it was 'false' before. And then one of them will trigger the WARN() in CPUFREQ_POSTCHANGE. Is there any protection in place and if so then how does it work? Rafael
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 6ca7eac..49d942a 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -108,7 +108,7 @@ static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list); static struct srcu_notifier_head cpufreq_transition_notifier_list; /* Tracks status of transition */ -static int transition_ongoing; +static bool transition_ongoing; static bool init_cpufreq_transition_notifier_list_called; static int __init init_cpufreq_transition_notifier_list(void) @@ -271,7 +271,7 @@ void __cpufreq_notify_transition(struct cpufreq_policy *policy, "In middle of another frequency transition\n")) return; - transition_ongoing++; + transition_ongoing = true; /* detect if the driver reported a value as "old frequency"