lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 13 Sep 2013 18:29:39 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	rjw@...k.pl
Cc:	linaro-kernel@...ts.linaro.org, patches@...aro.org,
	cpufreq@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Viresh Kumar <viresh.kumar@...aro.org>
Subject: [PATCH 2/2] cpufreq: powernow-k8: mark freq transition complete on error cases

Following patch "cpufreq: make sure frequency transitions are serialized"
guarantees that we don't have any races while changing cpu frequency or sending
notifications. It handled a special case with CPUFREQ_ASYNC_NOTIFICATION flag
for drivers that don't complete their freq change from ->target() and exynos5440
driver is well adopted to it as well..

There is one more driver powernow-k8 that has similar implementation, schedules
a work for doing transitions. All is good if that work function does
notifications on every call to it and so the transition_ongoing count stays
stable. But there are chances that the work function may return without actually
doing the notifications, in which case transition_ongoing count will not be set
to zero and so no transitions would be possible after that.

This patch fixes powernow-k8 driver to get transition_ongoing count stable. It
does following to ensure proper working of this driver:
- return -EINPROGRESS from ->target() so that core doesn't mark transfer over at
  the end of ->target().
- mark cpufreq_driver->flags with CPUFREQ_ASYNC_NOTIFICATION, so that core knows
  that driver would terminate its transition.
- call cpufreq_transition_complete() whenever we are returning before sending
  notifications

Hopefully things will work well after this patch, only compiled tested at my
end.

Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
---
 drivers/cpufreq/powernow-k8.c | 47 +++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 2344a9e..8215bbc 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -952,7 +952,7 @@ static int transition_frequency_fidvid(struct powernow_k8_data *data,
 	if ((data->currvid == vid) && (data->currfid == fid)) {
 		pr_debug("target matches current values (fid 0x%x, vid 0x%x)\n",
 			fid, vid);
-		return 0;
+		return -EALREADY;
 	}
 
 	pr_debug("cpu %d, changing to fid 0x%x, vid 0x%x\n",
@@ -993,22 +993,27 @@ static long powernowk8_target_fn(void *arg)
 	unsigned int newstate;
 	int ret;
 
-	if (!data)
-		return -EINVAL;
+	if (!data) {
+		ret = -EINVAL;
+		goto transition_complete;
+	}
 
 	checkfid = data->currfid;
 	checkvid = data->currvid;
 
 	if (pending_bit_stuck()) {
 		printk(KERN_ERR PFX "failing targ, change pending bit set\n");
-		return -EIO;
+		ret = -EIO;
+		goto transition_complete;
 	}
 
 	pr_debug("targ: cpu %d, %d kHz, min %d, max %d, relation %d\n",
 		pol->cpu, targfreq, pol->min, pol->max, relation);
 
-	if (query_current_values_with_pending_wait(data))
-		return -EIO;
+	if (query_current_values_with_pending_wait(data)) {
+		ret = -EIO;
+		goto transition_complete;
+	}
 
 	pr_debug("targ: curr fid 0x%x, vid 0x%x\n",
 		 data->currfid, data->currvid);
@@ -1022,25 +1027,36 @@ static long powernowk8_target_fn(void *arg)
 	}
 
 	if (cpufreq_frequency_table_target(pol, data->powernow_table,
-				targfreq, relation, &newstate))
-		return -EIO;
+				targfreq, relation, &newstate)) {
+		ret = -EIO;
+		goto transition_complete;
+	}
 
 	mutex_lock(&fidvid_mutex);
 
 	powernow_k8_acpi_pst_values(data, newstate);
 
 	ret = transition_frequency_fidvid(data, newstate);
+	mutex_unlock(&fidvid_mutex);
 
 	if (ret) {
-		printk(KERN_ERR PFX "transition frequency failed\n");
-		mutex_unlock(&fidvid_mutex);
-		return 1;
+		/* Not a error case, just need to mark transition complete */
+		if (ret == -EALREADY) {
+			ret = 0;
+		} else {
+			printk(KERN_ERR PFX "transition frequency failed\n");
+			ret = 1;
+		}
+		goto transition_complete;
 	}
-	mutex_unlock(&fidvid_mutex);
 
 	pol->cur = find_khz_freq_from_fid(data->currfid);
 
 	return 0;
+
+transition_complete:
+	cpufreq_transition_complete(pol);
+	return ret;
 }
 
 /* Driver entry point to switch to the target frequency */
@@ -1049,8 +1065,12 @@ static int powernowk8_target(struct cpufreq_policy *pol,
 {
 	struct powernowk8_target_arg pta = { .pol = pol, .targfreq = targfreq,
 					     .relation = relation };
+	int ret;
 
-	return work_on_cpu(pol->cpu, powernowk8_target_fn, &pta);
+	ret = work_on_cpu(pol->cpu, powernowk8_target_fn, &pta);
+	if (!ret)
+		ret = -EINPROGRESS;	/* Mark transition as In-progress */
+	return ret;
 }
 
 /* Driver entry point to verify the policy and range of frequencies */
@@ -1233,6 +1253,7 @@ static struct freq_attr *powernow_k8_attr[] = {
 };
 
 static struct cpufreq_driver cpufreq_amd64_driver = {
+	.flags		= CPUFREQ_ASYNC_NOTIFICATION,
 	.verify		= powernowk8_verify,
 	.target		= powernowk8_target,
 	.bios_limit	= acpi_processor_get_bios_limit,
-- 
1.7.12.rc2.18.g61b472e

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ