Commit 5a75c82828e7c088ca6e7b4827911dc29cc8e774
Committed by
Dave Jones
1 parent
37c90e8887
Exists in
master
and in
39 other branches
[CPUFREQ] Cleanup locking in ondemand governor
Redesign the locking inside ondemand driver. Make dbs_mutex handle all the global state changes inside the driver and invent a new percpu mutex to serialize percpu timer and frequency limit change. Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> Signed-off-by: Dave Jones <davej@redhat.com>
Showing 1 changed file with 27 additions and 35 deletions Side-by-side Diff
drivers/cpufreq/cpufreq_ondemand.c
... | ... | @@ -70,8 +70,13 @@ |
70 | 70 | unsigned int freq_lo_jiffies; |
71 | 71 | unsigned int freq_hi_jiffies; |
72 | 72 | int cpu; |
73 | - unsigned int enable:1, | |
74 | - sample_type:1; | |
73 | + unsigned int sample_type:1; | |
74 | + /* | |
75 | + * percpu mutex that serializes governor limit change with | |
76 | + * do_dbs_timer invocation. We do not want do_dbs_timer to run | |
77 | + * when user is changing the governor or limits. | |
78 | + */ | |
79 | + struct mutex timer_mutex; | |
75 | 80 | }; |
76 | 81 | static DEFINE_PER_CPU(struct cpu_dbs_info_s, cpu_dbs_info); |
77 | 82 | |
... | ... | @@ -79,9 +84,7 @@ |
79 | 84 | |
80 | 85 | /* |
81 | 86 | * dbs_mutex protects data in dbs_tuners_ins from concurrent changes on |
82 | - * different CPUs. It protects dbs_enable in governor start/stop. It also | |
83 | - * serializes governor limit_change with do_dbs_timer. We do not want | |
84 | - * do_dbs_timer to run when user is changing the governor or limits. | |
87 | + * different CPUs. It protects dbs_enable in governor start/stop. | |
85 | 88 | */ |
86 | 89 | static DEFINE_MUTEX(dbs_mutex); |
87 | 90 | |
88 | 91 | |
... | ... | @@ -187,13 +190,18 @@ |
187 | 190 | return freq_hi; |
188 | 191 | } |
189 | 192 | |
193 | +static void ondemand_powersave_bias_init_cpu(int cpu) | |
194 | +{ | |
195 | + struct cpu_dbs_info_s *dbs_info = &per_cpu(cpu_dbs_info, cpu); | |
196 | + dbs_info->freq_table = cpufreq_frequency_get_table(cpu); | |
197 | + dbs_info->freq_lo = 0; | |
198 | +} | |
199 | + | |
190 | 200 | static void ondemand_powersave_bias_init(void) |
191 | 201 | { |
192 | 202 | int i; |
193 | 203 | for_each_online_cpu(i) { |
194 | - struct cpu_dbs_info_s *dbs_info = &per_cpu(cpu_dbs_info, i); | |
195 | - dbs_info->freq_table = cpufreq_frequency_get_table(i); | |
196 | - dbs_info->freq_lo = 0; | |
204 | + ondemand_powersave_bias_init_cpu(i); | |
197 | 205 | } |
198 | 206 | } |
199 | 207 | |
200 | 208 | |
... | ... | @@ -235,12 +243,10 @@ |
235 | 243 | unsigned int input; |
236 | 244 | int ret; |
237 | 245 | ret = sscanf(buf, "%u", &input); |
246 | + if (ret != 1) | |
247 | + return -EINVAL; | |
238 | 248 | |
239 | 249 | mutex_lock(&dbs_mutex); |
240 | - if (ret != 1) { | |
241 | - mutex_unlock(&dbs_mutex); | |
242 | - return -EINVAL; | |
243 | - } | |
244 | 250 | dbs_tuners_ins.sampling_rate = max(input, min_sampling_rate); |
245 | 251 | mutex_unlock(&dbs_mutex); |
246 | 252 | |
247 | 253 | |
248 | 254 | |
... | ... | @@ -254,13 +260,12 @@ |
254 | 260 | int ret; |
255 | 261 | ret = sscanf(buf, "%u", &input); |
256 | 262 | |
257 | - mutex_lock(&dbs_mutex); | |
258 | 263 | if (ret != 1 || input > MAX_FREQUENCY_UP_THRESHOLD || |
259 | 264 | input < MIN_FREQUENCY_UP_THRESHOLD) { |
260 | - mutex_unlock(&dbs_mutex); | |
261 | 265 | return -EINVAL; |
262 | 266 | } |
263 | 267 | |
268 | + mutex_lock(&dbs_mutex); | |
264 | 269 | dbs_tuners_ins.up_threshold = input; |
265 | 270 | mutex_unlock(&dbs_mutex); |
266 | 271 | |
... | ... | @@ -358,9 +363,6 @@ |
358 | 363 | struct cpufreq_policy *policy; |
359 | 364 | unsigned int j; |
360 | 365 | |
361 | - if (!this_dbs_info->enable) | |
362 | - return; | |
363 | - | |
364 | 366 | this_dbs_info->freq_lo = 0; |
365 | 367 | policy = this_dbs_info->cur_policy; |
366 | 368 | |
367 | 369 | |
... | ... | @@ -488,14 +490,8 @@ |
488 | 490 | int delay = usecs_to_jiffies(dbs_tuners_ins.sampling_rate); |
489 | 491 | |
490 | 492 | delay -= jiffies % delay; |
493 | + mutex_lock(&dbs_info->timer_mutex); | |
491 | 494 | |
492 | - mutex_lock(&dbs_mutex); | |
493 | - | |
494 | - if (!dbs_info->enable) { | |
495 | - mutex_unlock(&dbs_mutex); | |
496 | - return; | |
497 | - } | |
498 | - | |
499 | 495 | /* Common NORMAL_SAMPLE setup */ |
500 | 496 | dbs_info->sample_type = DBS_NORMAL_SAMPLE; |
501 | 497 | if (!dbs_tuners_ins.powersave_bias || |
... | ... | @@ -511,7 +507,7 @@ |
511 | 507 | dbs_info->freq_lo, CPUFREQ_RELATION_H); |
512 | 508 | } |
513 | 509 | queue_delayed_work_on(cpu, kondemand_wq, &dbs_info->work, delay); |
514 | - mutex_unlock(&dbs_mutex); | |
510 | + mutex_unlock(&dbs_info->timer_mutex); | |
515 | 511 | } |
516 | 512 | |
517 | 513 | static inline void dbs_timer_init(struct cpu_dbs_info_s *dbs_info) |
... | ... | @@ -520,8 +516,6 @@ |
520 | 516 | int delay = usecs_to_jiffies(dbs_tuners_ins.sampling_rate); |
521 | 517 | delay -= jiffies % delay; |
522 | 518 | |
523 | - dbs_info->enable = 1; | |
524 | - ondemand_powersave_bias_init(); | |
525 | 519 | dbs_info->sample_type = DBS_NORMAL_SAMPLE; |
526 | 520 | INIT_DELAYED_WORK_DEFERRABLE(&dbs_info->work, do_dbs_timer); |
527 | 521 | queue_delayed_work_on(dbs_info->cpu, kondemand_wq, &dbs_info->work, |
... | ... | @@ -530,7 +524,6 @@ |
530 | 524 | |
531 | 525 | static inline void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info) |
532 | 526 | { |
533 | - dbs_info->enable = 0; | |
534 | 527 | cancel_delayed_work_sync(&dbs_info->work); |
535 | 528 | } |
536 | 529 | |
537 | 530 | |
538 | 531 | |
539 | 532 | |
... | ... | @@ -549,19 +542,15 @@ |
549 | 542 | if ((!cpu_online(cpu)) || (!policy->cur)) |
550 | 543 | return -EINVAL; |
551 | 544 | |
552 | - if (this_dbs_info->enable) /* Already enabled */ | |
553 | - break; | |
554 | - | |
555 | 545 | mutex_lock(&dbs_mutex); |
556 | - dbs_enable++; | |
557 | 546 | |
558 | 547 | rc = sysfs_create_group(&policy->kobj, &dbs_attr_group); |
559 | 548 | if (rc) { |
560 | - dbs_enable--; | |
561 | 549 | mutex_unlock(&dbs_mutex); |
562 | 550 | return rc; |
563 | 551 | } |
564 | 552 | |
553 | + dbs_enable++; | |
565 | 554 | for_each_cpu(j, policy->cpus) { |
566 | 555 | struct cpu_dbs_info_s *j_dbs_info; |
567 | 556 | j_dbs_info = &per_cpu(cpu_dbs_info, j); |
... | ... | @@ -575,6 +564,8 @@ |
575 | 564 | } |
576 | 565 | } |
577 | 566 | this_dbs_info->cpu = cpu; |
567 | + ondemand_powersave_bias_init_cpu(cpu); | |
568 | + mutex_init(&this_dbs_info->timer_mutex); | |
578 | 569 | /* |
579 | 570 | * Start the timerschedule work, when this governor |
580 | 571 | * is used for first time |
581 | 572 | |
582 | 573 | |
... | ... | @@ -602,20 +593,21 @@ |
602 | 593 | |
603 | 594 | mutex_lock(&dbs_mutex); |
604 | 595 | sysfs_remove_group(&policy->kobj, &dbs_attr_group); |
596 | + mutex_destroy(&this_dbs_info->timer_mutex); | |
605 | 597 | dbs_enable--; |
606 | 598 | mutex_unlock(&dbs_mutex); |
607 | 599 | |
608 | 600 | break; |
609 | 601 | |
610 | 602 | case CPUFREQ_GOV_LIMITS: |
611 | - mutex_lock(&dbs_mutex); | |
603 | + mutex_lock(&this_dbs_info->timer_mutex); | |
612 | 604 | if (policy->max < this_dbs_info->cur_policy->cur) |
613 | 605 | __cpufreq_driver_target(this_dbs_info->cur_policy, |
614 | 606 | policy->max, CPUFREQ_RELATION_H); |
615 | 607 | else if (policy->min > this_dbs_info->cur_policy->cur) |
616 | 608 | __cpufreq_driver_target(this_dbs_info->cur_policy, |
617 | 609 | policy->min, CPUFREQ_RELATION_L); |
618 | - mutex_unlock(&dbs_mutex); | |
610 | + mutex_unlock(&this_dbs_info->timer_mutex); | |
619 | 611 | break; |
620 | 612 | } |
621 | 613 | return 0; |