All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bálint Czobor" <czoborbalint@gmail.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	"Todd Poynor" <toddpoynor@google.com>,
	"Nicolas Pitre" <nicolas.pitre@linaro.org>,
	"Bálint Czobor" <czoborbalint@gmail.com>
Subject: [PATCH 35/70] cpufreq: interactive: fix racy timer stopping
Date: Tue, 27 Oct 2015 18:30:23 +0100	[thread overview]
Message-ID: <1445967059-6897-35-git-send-email-czoborbalint@gmail.com> (raw)
In-Reply-To: <1445967059-6897-1-git-send-email-czoborbalint@gmail.com>

From: Todd Poynor <toddpoynor@google.com>

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>
Signed-off-by: Bálint Czobor <czoborbalint@gmail.com>
---
 drivers/cpufreq/cpufreq_interactive.c |   38 ++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_interactive.c b/drivers/cpufreq/cpufreq_interactive.c
index f8e9ee9..74f5609 100644
--- a/drivers/cpufreq/cpufreq_interactive.c
+++ b/drivers/cpufreq/cpufreq_interactive.c
@@ -21,7 +21,7 @@
 #include <linux/cpufreq.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/mutex.h>
+#include <linux/rwsem.h>
 #include <linux/sched.h>
 #include <linux/sched/rt.h>
 #include <linux/tick.h>
@@ -29,7 +29,6 @@
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/kthread.h>
-#include <linux/mutex.h>
 #include <linux/slab.h>
 #include <asm/cputime.h>
 
@@ -52,6 +51,7 @@ struct cpufreq_interactive_cpuinfo {
 	unsigned int floor_freq;
 	u64 floor_validate_time;
 	u64 hispeed_validate_time;
+	struct rw_semaphore enable_sem;
 	int governor_enabled;
 };
 
@@ -279,8 +279,8 @@ static void cpufreq_interactive_timer(unsigned long data)
 	unsigned long flags;
 	bool boosted;
 
-	smp_rmb();
-
+	if (!down_read_trylock(&pcpu->enable_sem))
+		return;
 	if (!pcpu->governor_enabled)
 		goto exit;
 
@@ -387,6 +387,7 @@ rearm:
 		cpufreq_interactive_timer_resched(pcpu);
 
 exit:
+	up_read(&pcpu->enable_sem);
 	return;
 }
 
@@ -396,8 +397,12 @@ static void cpufreq_interactive_idle_start(void)
 		&per_cpu(cpuinfo, smp_processor_id());
 	int pending;
 
-	if (!pcpu->governor_enabled)
+	if (!down_read_trylock(&pcpu->enable_sem))
+		return;
+	if (!pcpu->governor_enabled) {
+		up_read(&pcpu->enable_sem);
 		return;
+	}
 
 	pending = timer_pending(&pcpu->cpu_timer);
 
@@ -414,6 +419,7 @@ static void cpufreq_interactive_idle_start(void)
 			cpufreq_interactive_timer_resched(pcpu);
 	}
 
+	up_read(&pcpu->enable_sem);
 }
 
 static void cpufreq_interactive_idle_end(void)
@@ -421,8 +427,12 @@ static void cpufreq_interactive_idle_end(void)
 	struct cpufreq_interactive_cpuinfo *pcpu =
 		&per_cpu(cpuinfo, smp_processor_id());
 
-	if (!pcpu->governor_enabled)
+	if (!down_read_trylock(&pcpu->enable_sem))
+		return;
+	if (!pcpu->governor_enabled) {
+		up_read(&pcpu->enable_sem);
 		return;
+	}
 
 	/* Arm the timer for 1-2 ticks later if not already. */
 	if (!timer_pending(&pcpu->cpu_timer)) {
@@ -432,6 +442,8 @@ static void cpufreq_interactive_idle_end(void)
 		del_timer(&pcpu->cpu_slack_timer);
 		cpufreq_interactive_timer(smp_processor_id());
 	}
+
+	up_read(&pcpu->enable_sem);
 }
 
 static int cpufreq_interactive_speedchange_task(void *data)
@@ -466,10 +478,12 @@ static int cpufreq_interactive_speedchange_task(void *data)
 			unsigned int max_freq = 0;
 
 			pcpu = &per_cpu(cpuinfo, cpu);
-			smp_rmb();
-
-			if (!pcpu->governor_enabled)
+			if (!down_read_trylock(&pcpu->enable_sem))
+				continue;
+			if (!pcpu->governor_enabled) {
+				up_read(&pcpu->enable_sem);
 				continue;
+			}
 
 			for_each_cpu(j, pcpu->policy->cpus) {
 				struct cpufreq_interactive_cpuinfo *pjcpu =
@@ -486,6 +500,8 @@ static int cpufreq_interactive_speedchange_task(void *data)
 			trace_cpufreq_interactive_setspeed(cpu,
 						     pcpu->target_freq,
 						     pcpu->policy->cur);
+
+			up_read(&pcpu->enable_sem);
 		}
 	}
 
@@ -936,10 +952,11 @@ static int cpufreq_governor_interactive(struct cpufreq_policy *policy,
 	case CPUFREQ_GOV_STOP:
 		for_each_cpu(j, policy->cpus) {
 			pcpu = &per_cpu(cpuinfo, j);
+			down_write(&pcpu->enable_sem);
 			pcpu->governor_enabled = 0;
-			smp_wmb();
 			del_timer_sync(&pcpu->cpu_timer);
 			del_timer_sync(&pcpu->cpu_slack_timer);
+			up_write(&pcpu->enable_sem);
 		}
 
 		if (atomic_dec_return(&active_count) > 0)
@@ -989,6 +1006,7 @@ static int __init cpufreq_interactive_init(void)
 		init_timer(&pcpu->cpu_slack_timer);
 		pcpu->cpu_slack_timer.function = cpufreq_interactive_nop_timer;
 		spin_lock_init(&pcpu->load_lock);
+		init_rwsem(&pcpu->enable_sem);
 	}
 
 	spin_lock_init(&target_loads_lock);
-- 
1.7.9.5


  parent reply	other threads:[~2015-10-27 17:32 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-27 17:29 [PATCH 01/70] cpufreq: interactive: New 'interactive' governor Bálint Czobor
2015-10-27 17:29 ` [PATCH 02/70] cpufreq interactive governor: event tracing Bálint Czobor
2015-10-27 17:29 ` [PATCH 03/70] cpufreq: interactive: apply intermediate load to max speed not current Bálint Czobor
2015-10-27 17:29 ` [PATCH 04/70] cpufreq: interactive: set at least hispeed when above hispeed load Bálint Czobor
2015-10-27 17:29 ` [PATCH 05/70] cpufreq: interactive: don't drop speed if recently at higher load Bálint Czobor
2015-10-27 17:29 ` [PATCH 06/70] cpufreq: interactive: configurable delay before raising above hispeed Bálint Czobor
2015-10-27 17:29 ` [PATCH 07/70] cpufreq: interactive: adjust code and documentation to match Bálint Czobor
2015-10-27 17:29 ` [PATCH 08/70] cpufreq: interactive: base hispeed bump on target freq, not actual Bálint Czobor
2015-10-27 17:29 ` [PATCH 09/70] cpufreq: interactive: Separate speed target revalidate time and initial set time Bálint Czobor
2015-10-27 17:29 ` [PATCH 10/70] cpufreq: interactive: Boost frequency on touchscreen input Bálint Czobor
2015-10-27 17:29 ` [PATCH 11/70] cpufreq: interactive: remove unused target_validate_time_in_idle Bálint Czobor
2015-10-27 17:30 ` [PATCH 12/70] cpufreq: interactive: Add sysfs boost interface for hints from userspace Bálint Czobor
2015-10-27 17:30 ` [PATCH 13/70] cpufreq: interactive: set floor for boosted speed Bálint Czobor
2015-10-27 17:30 ` [PATCH 14/70] cpufreq: interactive: add boost pulse interface Bálint Czobor
2015-10-27 17:30 ` [PATCH 15/70] cpufreq-interactive: Compile fixup Bálint Czobor
2015-10-27 17:30 ` [PATCH 16/70] cpufreq: interactive: restart above_hispeed_delay at each hispeed load Bálint Czobor
2015-10-27 17:30 ` [PATCH 17/70] cpufreq: interactive: take idle notifications only when active Bálint Czobor
2015-10-27 20:56   ` kbuild test robot
2015-10-27 20:56     ` kbuild test robot
2015-10-27 17:30 ` [PATCH 18/70] cpufreq: interactive: keep freezer happy when not current governor Bálint Czobor
2015-10-27 17:30 ` [PATCH 19/70] cpufreq: interactive: handle speed up and down in the realtime task Bálint Czobor
2015-10-27 17:30 ` [PATCH 20/70] cpufreq: interactive: remove input_boost handling Bálint Czobor
2015-10-27 17:30 ` [PATCH 21/70] cpufreq: interactive: always limit initial speed bump to hispeed Bálint Czobor
2015-10-27 17:30 ` [PATCH 22/70] cpufreq: interactive: run at fraction of hispeed_freq when load is low Bálint Czobor
2015-10-27 17:30 ` [PATCH 23/70] cpufreq: interactive: pin timers to associated CPU Bálint Czobor
2015-10-27 17:30 ` [PATCH 24/70] cpufreq: interactive: use deferrable timer by default Bálint Czobor
2015-10-27 17:30 ` [PATCH 25/70] cpufreq: interactive: kick timer on idle exit past expiry Bálint Czobor
2015-10-27 17:30 ` [PATCH 26/70] cpufreq: interactive: trace actual speed in target speed decisions Bálint Czobor
2015-10-27 17:30 ` [PATCH 27/70] cpufreq: interactive: change speed according to current speed and target load Bálint Czobor
2015-10-27 17:30 ` [PATCH 28/70] cpufreq: interactive: apply above_hispeed_delay to each step above hispeed Bálint Czobor
2015-10-27 17:30 ` [PATCH 29/70] cpufreq: interactive: allow arbitrary speed / target load mappings Bálint Czobor
2015-10-27 17:30 ` [PATCH 30/70] cpufreq: interactive: remove load since last speed change Bálint Czobor
2015-10-27 17:30 ` [PATCH 31/70] cpufreq: interactive: adjust load for changes in speed Bálint Czobor
2015-10-27 17:30 ` [PATCH 32/70] cpufreq: interactive: specify duration of CPU speed boost pulse Bálint Czobor
2015-10-27 17:30 ` [PATCH 33/70] cpufreq: interactive: add timer slack to limit idle at speed > min Bálint Czobor
2015-10-27 17:30 ` [PATCH 34/70] cpufreq: interactive: fix boosting logic Bálint Czobor
2015-10-27 17:30 ` Bálint Czobor [this message]
2015-10-27 17:30 ` [PATCH 36/70] cpufreq: interactive: fix race on timer restart on governor start Bálint Czobor
2015-10-27 17:30 ` [PATCH 37/70] cpufreq: interactive: default go_hispeed_load 99%, doc updates Bálint Czobor
2015-10-27 17:30 ` [PATCH 38/70] cpufreq: interactive: init default values at compile time Bálint Czobor
2015-10-27 17:30 ` [PATCH 39/70] cpufreq: interactive: don't handle transition notification if not enabled Bálint Czobor
2015-10-27 17:30 ` [PATCH 40/70] cpufreq: interactive: fix deadlock on spinlock in timer Bálint Czobor
2015-10-27 17:30 ` [PATCH 41/70] cpufreq: interactive: fix race on governor start/stop Bálint Czobor
2015-10-27 17:30 ` [PATCH 42/70] cpufreq: interactive: allow arbitrary speed / delay mappings Bálint Czobor
2015-10-27 17:30 ` [PATCH 43/70] cpufreq: interactive: add io_is_busy interface Bálint Czobor
2015-10-27 17:30 ` [PATCH 44/70] cpufreq: interactive: fix crash on error paths in get_tokenized_data Bálint Czobor
2015-10-27 17:30 ` [PATCH 45/70] cpufreq: interactive: base above_hispeed_delay on target freq, not current Bálint Czobor
2015-10-27 17:30 ` [PATCH 46/70] cpufreq: interactive: fix uninitialized spinlock Bálint Czobor
2015-10-27 17:30 ` [PATCH 47/70] cpufreq: interactive: handle errors from cpufreq_frequency_table_target Bálint Czobor
2015-10-27 17:30 ` [PATCH 48/70] cpufreq: interactive: reduce chance of zero time delta on load eval Bálint Czobor
2015-10-27 17:30 ` [PATCH 49/70] cpufreq: interactive: avoid underflow on active time calculation Bálint Czobor
2015-10-27 17:30 ` [PATCH 50/70] cpufreq: interactive: fix race on cpufreq TRANSITION notifier Bálint Czobor
2015-10-27 17:30 ` [PATCH 51/70] cpufreq: interactive: resched timer if max freq raised Bálint Czobor
2015-10-27 17:30 ` [PATCH 52/70] cpufreq: interactive: fix show_target_loads and show_above_hispeed_delay Bálint Czobor
2015-10-27 17:30 ` [PATCH 53/70] cpufreq: interactive: Remove unnecessary cpu_online() check Bálint Czobor
2015-10-27 17:30 ` [PATCH 54/70] cpufreq: interactive: Move definition of cpufreq_gov_interactive downwards Bálint Czobor
2015-10-27 17:30 ` [PATCH 55/70] cpufreq: Interactive: Implement per policy instances of governor Bálint Czobor
2015-10-27 17:30 ` [PATCH 56/70] cpufreq: interactive: delete timers for GOV_START Bálint Czobor
2015-10-27 17:30 ` [PATCH 57/70] cpufreq: interactive: fix compiling warnings Bálint Czobor
2015-10-27 17:30 ` [PATCH 58/70] cpufreq: interactive: fix NULL pointer dereference at sysfs ops Bálint Czobor
2015-10-27 17:30 ` [PATCH 59/70] cpufreq: interactive: Use generic get_cpu_idle_time() from cpufreq.c Bálint Czobor
2015-10-27 17:30 ` [PATCH 60/70] cpufreq: interactive: hold reference on global cpufreq kobject if needed Bálint Czobor
2015-10-27 17:30 ` [PATCH 61/70] cpufreq: interactive: restructure CPUFREQ_GOV_LIMITS Bálint Czobor
2015-10-27 17:30 ` [PATCH 62/70] cpufreq: interactive: turn boost_pulse off on boost off Bálint Czobor
2015-10-27 17:30 ` [PATCH 63/70] cpufreq: interactive: remove compilation error from commit 49cc72365fb7ee87762a7ccc6a32ef68627216c5 Bálint Czobor
2015-10-27 17:30 ` [PATCH 64/70] cpufreq: interactive: prevents the frequency to directly raise above the hispeed_freq from a lower frequency Bálint Czobor
2015-10-27 17:30 ` [PATCH 65/70] cpufreq: interactive: make common_tunables static Bálint Czobor
2015-10-27 17:30 ` [PATCH 66/70] cpufreq: interactive: Fix compile errors in accordance with changes from 3.14 to 3.18 Bálint Czobor
2015-10-28  2:15   ` kbuild test robot
2015-10-28  2:15     ` kbuild test robot
2015-10-27 17:30 ` [PATCH 67/70] cpufreq: interactive: Put global cpufreq kobject on failure Bálint Czobor
2015-10-27 17:30 ` [PATCH 68/70] subsystem: CPU FREQUENCY DRIVERS- Set cpu_load calculation on current frequency Bálint Czobor
2015-10-27 17:30 ` [PATCH 69/70] cpufreq: interactive: Don't set floor_validate_time during boost Bálint Czobor
2015-10-27 17:30 ` [PATCH 70/70] cpufreq: interactive: Round up timer_rate to match jiffy Bálint Czobor
2015-10-27 20:44 ` [PATCH 01/70] cpufreq: interactive: New 'interactive' governor kbuild test robot
2015-10-27 20:44   ` kbuild test robot
2015-10-28  0:59 ` Rafael J. Wysocki
2015-10-28  3:00   ` Viresh Kumar
     [not found]     ` <CAL6DFdfsDTtkkpLJ7kg9L9rFh+Ho6DC7x9a+F3x9DydvXHfbPQ@mail.gmail.com>
     [not found]       ` <CAL6DFdeSbzM5go==i8Z_CJ19VrAMWVVWF5NuD+VNMONaPK5zLA@mail.gmail.com>
2015-10-29  1:42         ` Viresh Kumar
2015-11-02 14:02     ` Peter Zijlstra
2015-11-02 18:06       ` Steve Muckle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1445967059-6897-35-git-send-email-czoborbalint@gmail.com \
    --to=czoborbalint@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nicolas.pitre@linaro.org \
    --cc=rjw@rjwysocki.net \
    --cc=toddpoynor@google.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.