diff options
author | Todd Poynor | 2012-12-18 19:50:44 -0600 |
---|---|---|
committer | Arve Hjønnevåg | 2013-03-11 17:25:13 -0500 |
commit | a8c472d5f619cb40c620c84100856c50afd725d6 (patch) | |
tree | cc12f0a5d7e441981f53c8f0b2b293da14e95ba2 | |
parent | 4097df1d83f9ad28fe3bd9941859351f54a5f2b3 (diff) | |
download | kernel-common-a8c472d5f619cb40c620c84100856c50afd725d6.tar.gz kernel-common-a8c472d5f619cb40c620c84100856c50afd725d6.tar.xz kernel-common-a8c472d5f619cb40c620c84100856c50afd725d6.zip |
cpufreq: interactive: fix racy timer stopping
When stopping the governor, del_timer_sync() can race against an
invocation of the idle notifier callback, which has the potential
to reactivate the timer.
To fix this issue, a read-write semaphore is used. Multiple readers are
allowed as long as pcpu->governor_enabled is true. However it can be
moved to false only after taking a write semaphore which would wait for
any on-going asynchronous activities to complete and prevent any more of
those activities to be initiated.
[toddpoynor@google.com: cosmetic and commit text changes]
Change-Id: Ib51165a735d73dcf964a06754c48bdc1913e13d0
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
-rw-r--r-- | drivers/cpufreq/cpufreq_interactive.c | 38 |
1 files changed, 28 insertions, 10 deletions
diff --git a/drivers/cpufreq/cpufreq_interactive.c b/drivers/cpufreq/cpufreq_interactive.c index f8e9ee9f713..74f56093d2f 100644 --- a/drivers/cpufreq/cpufreq_interactive.c +++ b/drivers/cpufreq/cpufreq_interactive.c | |||
@@ -21,7 +21,7 @@ | |||
21 | #include <linux/cpufreq.h> | 21 | #include <linux/cpufreq.h> |
22 | #include <linux/module.h> | 22 | #include <linux/module.h> |
23 | #include <linux/moduleparam.h> | 23 | #include <linux/moduleparam.h> |
24 | #include <linux/mutex.h> | 24 | #include <linux/rwsem.h> |
25 | #include <linux/sched.h> | 25 | #include <linux/sched.h> |
26 | #include <linux/sched/rt.h> | 26 | #include <linux/sched/rt.h> |
27 | #include <linux/tick.h> | 27 | #include <linux/tick.h> |
@@ -29,7 +29,6 @@ | |||
29 | #include <linux/timer.h> | 29 | #include <linux/timer.h> |
30 | #include <linux/workqueue.h> | 30 | #include <linux/workqueue.h> |
31 | #include <linux/kthread.h> | 31 | #include <linux/kthread.h> |
32 | #include <linux/mutex.h> | ||
33 | #include <linux/slab.h> | 32 | #include <linux/slab.h> |
34 | #include <asm/cputime.h> | 33 | #include <asm/cputime.h> |
35 | 34 | ||
@@ -52,6 +51,7 @@ struct cpufreq_interactive_cpuinfo { | |||
52 | unsigned int floor_freq; | 51 | unsigned int floor_freq; |
53 | u64 floor_validate_time; | 52 | u64 floor_validate_time; |
54 | u64 hispeed_validate_time; | 53 | u64 hispeed_validate_time; |
54 | struct rw_semaphore enable_sem; | ||
55 | int governor_enabled; | 55 | int governor_enabled; |
56 | }; | 56 | }; |
57 | 57 | ||
@@ -279,8 +279,8 @@ static void cpufreq_interactive_timer(unsigned long data) | |||
279 | unsigned long flags; | 279 | unsigned long flags; |
280 | bool boosted; | 280 | bool boosted; |
281 | 281 | ||
282 | smp_rmb(); | 282 | if (!down_read_trylock(&pcpu->enable_sem)) |
283 | 283 | return; | |
284 | if (!pcpu->governor_enabled) | 284 | if (!pcpu->governor_enabled) |
285 | goto exit; | 285 | goto exit; |
286 | 286 | ||
@@ -387,6 +387,7 @@ rearm: | |||
387 | cpufreq_interactive_timer_resched(pcpu); | 387 | cpufreq_interactive_timer_resched(pcpu); |
388 | 388 | ||
389 | exit: | 389 | exit: |
390 | up_read(&pcpu->enable_sem); | ||
390 | return; | 391 | return; |
391 | } | 392 | } |
392 | 393 | ||
@@ -396,8 +397,12 @@ static void cpufreq_interactive_idle_start(void) | |||
396 | &per_cpu(cpuinfo, smp_processor_id()); | 397 | &per_cpu(cpuinfo, smp_processor_id()); |
397 | int pending; | 398 | int pending; |
398 | 399 | ||
399 | if (!pcpu->governor_enabled) | 400 | if (!down_read_trylock(&pcpu->enable_sem)) |
401 | return; | ||
402 | if (!pcpu->governor_enabled) { | ||
403 | up_read(&pcpu->enable_sem); | ||
400 | return; | 404 | return; |
405 | } | ||
401 | 406 | ||
402 | pending = timer_pending(&pcpu->cpu_timer); | 407 | pending = timer_pending(&pcpu->cpu_timer); |
403 | 408 | ||
@@ -414,6 +419,7 @@ static void cpufreq_interactive_idle_start(void) | |||
414 | cpufreq_interactive_timer_resched(pcpu); | 419 | cpufreq_interactive_timer_resched(pcpu); |
415 | } | 420 | } |
416 | 421 | ||
422 | up_read(&pcpu->enable_sem); | ||
417 | } | 423 | } |
418 | 424 | ||
419 | static void cpufreq_interactive_idle_end(void) | 425 | static void cpufreq_interactive_idle_end(void) |
@@ -421,8 +427,12 @@ static void cpufreq_interactive_idle_end(void) | |||
421 | struct cpufreq_interactive_cpuinfo *pcpu = | 427 | struct cpufreq_interactive_cpuinfo *pcpu = |
422 | &per_cpu(cpuinfo, smp_processor_id()); | 428 | &per_cpu(cpuinfo, smp_processor_id()); |
423 | 429 | ||
424 | if (!pcpu->governor_enabled) | 430 | if (!down_read_trylock(&pcpu->enable_sem)) |
431 | return; | ||
432 | if (!pcpu->governor_enabled) { | ||
433 | up_read(&pcpu->enable_sem); | ||
425 | return; | 434 | return; |
435 | } | ||
426 | 436 | ||
427 | /* Arm the timer for 1-2 ticks later if not already. */ | 437 | /* Arm the timer for 1-2 ticks later if not already. */ |
428 | if (!timer_pending(&pcpu->cpu_timer)) { | 438 | if (!timer_pending(&pcpu->cpu_timer)) { |
@@ -432,6 +442,8 @@ static void cpufreq_interactive_idle_end(void) | |||
432 | del_timer(&pcpu->cpu_slack_timer); | 442 | del_timer(&pcpu->cpu_slack_timer); |
433 | cpufreq_interactive_timer(smp_processor_id()); | 443 | cpufreq_interactive_timer(smp_processor_id()); |
434 | } | 444 | } |
445 | |||
446 | up_read(&pcpu->enable_sem); | ||
435 | } | 447 | } |
436 | 448 | ||
437 | static int cpufreq_interactive_speedchange_task(void *data) | 449 | static int cpufreq_interactive_speedchange_task(void *data) |
@@ -466,10 +478,12 @@ static int cpufreq_interactive_speedchange_task(void *data) | |||
466 | unsigned int max_freq = 0; | 478 | unsigned int max_freq = 0; |
467 | 479 | ||
468 | pcpu = &per_cpu(cpuinfo, cpu); | 480 | pcpu = &per_cpu(cpuinfo, cpu); |
469 | smp_rmb(); | 481 | if (!down_read_trylock(&pcpu->enable_sem)) |
470 | 482 | continue; | |
471 | if (!pcpu->governor_enabled) | 483 | if (!pcpu->governor_enabled) { |
484 | up_read(&pcpu->enable_sem); | ||
472 | continue; | 485 | continue; |
486 | } | ||
473 | 487 | ||
474 | for_each_cpu(j, pcpu->policy->cpus) { | 488 | for_each_cpu(j, pcpu->policy->cpus) { |
475 | struct cpufreq_interactive_cpuinfo *pjcpu = | 489 | struct cpufreq_interactive_cpuinfo *pjcpu = |
@@ -486,6 +500,8 @@ static int cpufreq_interactive_speedchange_task(void *data) | |||
486 | trace_cpufreq_interactive_setspeed(cpu, | 500 | trace_cpufreq_interactive_setspeed(cpu, |
487 | pcpu->target_freq, | 501 | pcpu->target_freq, |
488 | pcpu->policy->cur); | 502 | pcpu->policy->cur); |
503 | |||
504 | up_read(&pcpu->enable_sem); | ||
489 | } | 505 | } |
490 | } | 506 | } |
491 | 507 | ||
@@ -936,10 +952,11 @@ static int cpufreq_governor_interactive(struct cpufreq_policy *policy, | |||
936 | case CPUFREQ_GOV_STOP: | 952 | case CPUFREQ_GOV_STOP: |
937 | for_each_cpu(j, policy->cpus) { | 953 | for_each_cpu(j, policy->cpus) { |
938 | pcpu = &per_cpu(cpuinfo, j); | 954 | pcpu = &per_cpu(cpuinfo, j); |
955 | down_write(&pcpu->enable_sem); | ||
939 | pcpu->governor_enabled = 0; | 956 | pcpu->governor_enabled = 0; |
940 | smp_wmb(); | ||
941 | del_timer_sync(&pcpu->cpu_timer); | 957 | del_timer_sync(&pcpu->cpu_timer); |
942 | del_timer_sync(&pcpu->cpu_slack_timer); | 958 | del_timer_sync(&pcpu->cpu_slack_timer); |
959 | up_write(&pcpu->enable_sem); | ||
943 | } | 960 | } |
944 | 961 | ||
945 | if (atomic_dec_return(&active_count) > 0) | 962 | if (atomic_dec_return(&active_count) > 0) |
@@ -989,6 +1006,7 @@ static int __init cpufreq_interactive_init(void) | |||
989 | init_timer(&pcpu->cpu_slack_timer); | 1006 | init_timer(&pcpu->cpu_slack_timer); |
990 | pcpu->cpu_slack_timer.function = cpufreq_interactive_nop_timer; | 1007 | pcpu->cpu_slack_timer.function = cpufreq_interactive_nop_timer; |
991 | spin_lock_init(&pcpu->load_lock); | 1008 | spin_lock_init(&pcpu->load_lock); |
1009 | init_rwsem(&pcpu->enable_sem); | ||
992 | } | 1010 | } |
993 | 1011 | ||
994 | spin_lock_init(&target_loads_lock); | 1012 | spin_lock_init(&target_loads_lock); |