diff mbox

[3/3] cpuidle: coupled: fix race condition between pokes and safe state

Message ID 1377287112-12018-3-git-send-email-ccross@android.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Colin Cross Aug. 23, 2013, 7:45 p.m. UTC
The coupled cpuidle waiting loop clears pending pokes before
entering the safe state.  If a poke arrives just before the
pokes are cleared, but after the while loop condition checks,
the poke will be lost and the cpu will stay in the safe state
until another interrupt arrives.  This may cause the cpu that
sent the poke to spin in the ready loop with interrupts off
until another cpu receives an interrupt, and if no other cpus
have interrupts routed to them it can spin forever.

Change the return value of cpuidle_coupled_clear_pokes to
return if a poke was cleared, and move the need_resched()
checks into the callers.  In the waiting loop, if
a poke was cleared restart the loop to repeat the while
condition checks.

Reported-by: Neil Zhang <zhangwm@marvell.com>
CC: stable@vger.kernel.org
Signed-off-by: Colin Cross <ccross@android.com>
---
 drivers/cpuidle/coupled.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Neil Zhang Aug. 27, 2013, 8:57 a.m. UTC | #1
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBDb2xpbiBDcm9zcyBbbWFpbHRv
OmNjcm9zc0BhbmRyb2lkLmNvbV0NCj4gU2VudDogMjAxM8TqONTCMjTI1SAzOjQ1DQo+IFRvOiBs
aW51eC1wbUB2Z2VyLmtlcm5lbC5vcmcNCj4gQ2M6IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5v
cmc7IE5laWwgWmhhbmc7IEpvc2VwaCBMbzsNCj4gbGludXgtdGVncmFAdmdlci5rZXJuZWwub3Jn
OyBDb2xpbiBDcm9zczsgc3RhYmxlQHZnZXIua2VybmVsLm9yZzsgUmFmYWVsIEouDQo+IFd5c29j
a2k7IERhbmllbCBMZXpjYW5vDQo+IFN1YmplY3Q6IFtQQVRDSCAzLzNdIGNwdWlkbGU6IGNvdXBs
ZWQ6IGZpeCByYWNlIGNvbmRpdGlvbiBiZXR3ZWVuIHBva2VzIGFuZCBzYWZlDQo+IHN0YXRlDQo+
IA0KPiBUaGUgY291cGxlZCBjcHVpZGxlIHdhaXRpbmcgbG9vcCBjbGVhcnMgcGVuZGluZyBwb2tl
cyBiZWZvcmUgZW50ZXJpbmcgdGhlIHNhZmUNCj4gc3RhdGUuICBJZiBhIHBva2UgYXJyaXZlcyBq
dXN0IGJlZm9yZSB0aGUgcG9rZXMgYXJlIGNsZWFyZWQsIGJ1dCBhZnRlciB0aGUgd2hpbGUNCj4g
bG9vcCBjb25kaXRpb24gY2hlY2tzLCB0aGUgcG9rZSB3aWxsIGJlIGxvc3QgYW5kIHRoZSBjcHUg
d2lsbCBzdGF5IGluIHRoZSBzYWZlIHN0YXRlDQo+IHVudGlsIGFub3RoZXIgaW50ZXJydXB0IGFy
cml2ZXMuICBUaGlzIG1heSBjYXVzZSB0aGUgY3B1IHRoYXQgc2VudCB0aGUgcG9rZSB0bw0KPiBz
cGluIGluIHRoZSByZWFkeSBsb29wIHdpdGggaW50ZXJydXB0cyBvZmYgdW50aWwgYW5vdGhlciBj
cHUgcmVjZWl2ZXMgYW4gaW50ZXJydXB0LA0KPiBhbmQgaWYgbm8gb3RoZXIgY3B1cyBoYXZlIGlu
dGVycnVwdHMgcm91dGVkIHRvIHRoZW0gaXQgY2FuIHNwaW4gZm9yZXZlci4NCj4gDQo+IENoYW5n
ZSB0aGUgcmV0dXJuIHZhbHVlIG9mIGNwdWlkbGVfY291cGxlZF9jbGVhcl9wb2tlcyB0byByZXR1
cm4gaWYgYSBwb2tlIHdhcw0KPiBjbGVhcmVkLCBhbmQgbW92ZSB0aGUgbmVlZF9yZXNjaGVkKCkg
Y2hlY2tzIGludG8gdGhlIGNhbGxlcnMuICBJbiB0aGUgd2FpdGluZw0KPiBsb29wLCBpZiBhIHBv
a2Ugd2FzIGNsZWFyZWQgcmVzdGFydCB0aGUgbG9vcCB0byByZXBlYXQgdGhlIHdoaWxlIGNvbmRp
dGlvbiBjaGVja3MuDQo+IA0KPiBSZXBvcnRlZC1ieTogTmVpbCBaaGFuZyA8emhhbmd3bUBtYXJ2
ZWxsLmNvbT4NCj4gQ0M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcNCj4gU2lnbmVkLW9mZi1ieTog
Q29saW4gQ3Jvc3MgPGNjcm9zc0BhbmRyb2lkLmNvbT4NCj4gLS0tDQo+ICBkcml2ZXJzL2NwdWlk
bGUvY291cGxlZC5jIHwgMjAgKysrKysrKysrKysrKystLS0tLS0NCj4gIDEgZmlsZSBjaGFuZ2Vk
LCAxNCBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvY3B1aWRsZS9jb3VwbGVkLmMgYi9kcml2ZXJzL2NwdWlkbGUvY291cGxlZC5jIGluZGV4
DQo+IGJiYzRiYTUuLmM5MTIzMGIgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvY3B1aWRsZS9jb3Vw
bGVkLmMNCj4gKysrIGIvZHJpdmVycy9jcHVpZGxlL2NvdXBsZWQuYw0KPiBAQCAtNDA4LDE5ICs0
MDgsMjIgQEAgc3RhdGljIHZvaWQgY3B1aWRsZV9jb3VwbGVkX3NldF9kb25lKGludCBjcHUsIHN0
cnVjdA0KPiBjcHVpZGxlX2NvdXBsZWQgKmNvdXBsZWQpDQo+ICAgKiBiZWVuIHByb2Nlc3NlZCBh
bmQgdGhlIHBva2UgYml0IGhhcyBiZWVuIGNsZWFyZWQuDQo+ICAgKg0KPiAgICogT3RoZXIgaW50
ZXJydXB0cyBtYXkgYWxzbyBiZSBwcm9jZXNzZWQgd2hpbGUgaW50ZXJydXB0cyBhcmUgZW5hYmxl
ZCwgc28NCj4gLSAqIG5lZWRfcmVzY2hlZCgpIG11c3QgYmUgdGVzdGVkIGFmdGVyIHR1cm5pbmcg
aW50ZXJydXB0cyBvZmYgYWdhaW4gdG8gbWFrZSBzdXJlDQo+ICsgKiBuZWVkX3Jlc2NoZWQoKSBt
dXN0IGJlIHRlc3RlZCBhZnRlciB0aGlzIGZ1bmN0aW9uIHJldHVybnMgdG8gbWFrZQ0KPiArIHN1
cmUNCj4gICAqIHRoZSBpbnRlcnJ1cHQgZGlkbid0IHNjaGVkdWxlIHdvcmsgdGhhdCBzaG91bGQg
dGFrZSB0aGUgY3B1IG91dCBvZiBpZGxlLg0KPiAgICoNCj4gLSAqIFJldHVybnMgMCBpZiBuZWVk
X3Jlc2NoZWQgd2FzIGZhbHNlLCAtRUlOVFIgaWYgbmVlZF9yZXNjaGVkIHdhcyB0cnVlLg0KPiAr
ICogUmV0dXJucyAwIGlmIG5vIHBva2Ugd2FzIHBlbmRpbmcsIDEgaWYgYSBwb2tlIHdhcyBjbGVh
cmVkLg0KPiAgICovDQo+ICBzdGF0aWMgaW50IGNwdWlkbGVfY291cGxlZF9jbGVhcl9wb2tlcyhp
bnQgY3B1KSAgew0KPiArCWlmICghY3B1bWFza190ZXN0X2NwdShjcHUsICZjcHVpZGxlX2NvdXBs
ZWRfcG9rZV9wZW5kaW5nKSkNCj4gKwkJcmV0dXJuIDA7DQo+ICsNCj4gIAlsb2NhbF9pcnFfZW5h
YmxlKCk7DQo+ICAJd2hpbGUgKGNwdW1hc2tfdGVzdF9jcHUoY3B1LCAmY3B1aWRsZV9jb3VwbGVk
X3Bva2VfcGVuZGluZykpDQo+ICAJCWNwdV9yZWxheCgpOw0KPiAgCWxvY2FsX2lycV9kaXNhYmxl
KCk7DQo+IA0KPiAtCXJldHVybiBuZWVkX3Jlc2NoZWQoKSA/IC1FSU5UUiA6IDA7DQo+ICsJcmV0
dXJuIDE7DQo+ICB9DQo+IA0KPiAgc3RhdGljIGJvb2wgY3B1aWRsZV9jb3VwbGVkX2FueV9wb2tl
c19wZW5kaW5nKHN0cnVjdCBjcHVpZGxlX2NvdXBsZWQNCj4gKmNvdXBsZWQpIEBAIC00NjQsNyAr
NDY3LDggQEAgaW50IGNwdWlkbGVfZW50ZXJfc3RhdGVfY291cGxlZChzdHJ1Y3QNCj4gY3B1aWRs
ZV9kZXZpY2UgKmRldiwNCj4gIAkJcmV0dXJuIC1FSU5WQUw7DQo+IA0KPiAgCXdoaWxlIChjb3Vw
bGVkLT5wcmV2ZW50KSB7DQo+IC0JCWlmIChjcHVpZGxlX2NvdXBsZWRfY2xlYXJfcG9rZXMoZGV2
LT5jcHUpKSB7DQo+ICsJCWNwdWlkbGVfY291cGxlZF9jbGVhcl9wb2tlcyhkZXYtPmNwdSk7DQo+
ICsJCWlmIChuZWVkX3Jlc2NoZWQoKSkgew0KPiAgCQkJbG9jYWxfaXJxX2VuYWJsZSgpOw0KPiAg
CQkJcmV0dXJuIGVudGVyZWRfc3RhdGU7DQo+ICAJCX0NCj4gQEAgLTUwMyw3ICs1MDcsMTAgQEAg
cmV0cnk6DQo+ICAJICovDQo+ICAJd2hpbGUgKCFjcHVpZGxlX2NvdXBsZWRfY3B1c193YWl0aW5n
KGNvdXBsZWQpIHx8DQo+ICAJCQkhY3B1bWFza190ZXN0X2NwdShkZXYtPmNwdSwgJmNwdWlkbGVf
Y291cGxlZF9wb2tlZCkpIHsNCj4gLQkJaWYgKGNwdWlkbGVfY291cGxlZF9jbGVhcl9wb2tlcyhk
ZXYtPmNwdSkpIHsNCj4gKwkJaWYgKGNwdWlkbGVfY291cGxlZF9jbGVhcl9wb2tlcyhkZXYtPmNw
dSkpDQo+ICsJCQljb250aW51ZTsNCj4gKw0KPiArCQlpZiAobmVlZF9yZXNjaGVkKCkpIHsNCj4g
IAkJCWNwdWlkbGVfY291cGxlZF9zZXRfbm90X3dhaXRpbmcoZGV2LT5jcHUsIGNvdXBsZWQpOw0K
PiAgCQkJZ290byBvdXQ7DQo+ICAJCX0NCj4gQEAgLTUxOCw3ICs1MjUsOCBAQCByZXRyeToNCj4g
IAkJbG9jYWxfaXJxX2Rpc2FibGUoKTsNCj4gIAl9DQo+IA0KPiAtCWlmIChjcHVpZGxlX2NvdXBs
ZWRfY2xlYXJfcG9rZXMoZGV2LT5jcHUpKSB7DQo+ICsJY3B1aWRsZV9jb3VwbGVkX2NsZWFyX3Bv
a2VzKGRldi0+Y3B1KTsNCj4gKwlpZiAobmVlZF9yZXNjaGVkKCkpIHsNCj4gIAkJY3B1aWRsZV9j
b3VwbGVkX3NldF9ub3Rfd2FpdGluZyhkZXYtPmNwdSwgY291cGxlZCk7DQo+ICAJCWdvdG8gb3V0
Ow0KPiAgCX0NCj4gLS0NCj4gMS44LjMNCg0KSSB0aGluayB0aGlzIHBhdGNoIHNob3VsZCBhbHNv
IGJlIHdvcmthYmxlLg0KVGhhbmtzLg0KDQpSZXZpZXdlZC1ieTogTmVpbCBaaGFuZyA8emhhbmd3
bUBtYXJ2ZWxsLmNvbT4NCg0KQmVzdCBSZWdhcmRzLA0KTmVpbCBaaGFuZw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index bbc4ba5..c91230b 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -408,19 +408,22 @@  static void cpuidle_coupled_set_done(int cpu, struct cpuidle_coupled *coupled)
  * been processed and the poke bit has been cleared.
  *
  * Other interrupts may also be processed while interrupts are enabled, so
- * need_resched() must be tested after turning interrupts off again to make sure
+ * need_resched() must be tested after this function returns to make sure
  * the interrupt didn't schedule work that should take the cpu out of idle.
  *
- * Returns 0 if need_resched was false, -EINTR if need_resched was true.
+ * Returns 0 if no poke was pending, 1 if a poke was cleared.
  */
 static int cpuidle_coupled_clear_pokes(int cpu)
 {
+	if (!cpumask_test_cpu(cpu, &cpuidle_coupled_poke_pending))
+		return 0;
+
 	local_irq_enable();
 	while (cpumask_test_cpu(cpu, &cpuidle_coupled_poke_pending))
 		cpu_relax();
 	local_irq_disable();
 
-	return need_resched() ? -EINTR : 0;
+	return 1;
 }
 
 static bool cpuidle_coupled_any_pokes_pending(struct cpuidle_coupled *coupled)
@@ -464,7 +467,8 @@  int cpuidle_enter_state_coupled(struct cpuidle_device *dev,
 		return -EINVAL;
 
 	while (coupled->prevent) {
-		if (cpuidle_coupled_clear_pokes(dev->cpu)) {
+		cpuidle_coupled_clear_pokes(dev->cpu);
+		if (need_resched()) {
 			local_irq_enable();
 			return entered_state;
 		}
@@ -503,7 +507,10 @@  retry:
 	 */
 	while (!cpuidle_coupled_cpus_waiting(coupled) ||
 			!cpumask_test_cpu(dev->cpu, &cpuidle_coupled_poked)) {
-		if (cpuidle_coupled_clear_pokes(dev->cpu)) {
+		if (cpuidle_coupled_clear_pokes(dev->cpu))
+			continue;
+
+		if (need_resched()) {
 			cpuidle_coupled_set_not_waiting(dev->cpu, coupled);
 			goto out;
 		}
@@ -518,7 +525,8 @@  retry:
 		local_irq_disable();
 	}
 
-	if (cpuidle_coupled_clear_pokes(dev->cpu)) {
+	cpuidle_coupled_clear_pokes(dev->cpu);
+	if (need_resched()) {
 		cpuidle_coupled_set_not_waiting(dev->cpu, coupled);
 		goto out;
 	}