Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1175027 > unrolled thread

Re:[PATCH] sched: fix BUG in preempt_notifier

Started bycfme_admin <cfme_admin@163.com>
First post2015-07-01 09:20 +0200
Last post2015-07-01 09:30 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re:[PATCH] sched: fix BUG in preempt_notifier cfme_admin <cfme_admin@163.com> - 2015-07-01 09:20 +0200
    Re: [PATCH] sched: fix BUG in preempt_notifier Ingo Molnar <mingo@kernel.org> - 2015-07-01 09:30 +0200

#1175027 — Re:[PATCH] sched: fix BUG in preempt_notifier

Fromcfme_admin <cfme_admin@163.com>
Date2015-07-01 09:20 +0200
SubjectRe:[PATCH] sched: fix BUG in preempt_notifier
Message-ID<pHktk-24l-15@gated-at.bofh.it>
Cgptb3JlIGluZm9ybWF0aW9uIGFib3V0IHRoZSBidWcgaXMgYmVsb3c6Cmh0dHBzOi8vYnVnemls
bGEua2VybmVsLm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAwNzAxCgp0aGUgcHJlZW1wdF9ub3RpZmll
cl91bnJlZ2lzdGVyIGZ1bmN0aW9uIGlzIHJ1biBpbl9hdG9taWMgY29udGV4dCwgYnV0IHN0YXRp
Y19rZXlfc2xvd19kZWMgZnVuY3Rpb24gbWF5IGNhdXNlIHNsZWVwLgoKCkF0IDIwMTUtMDctMDEg
MTQ6MzM6MzAsICJMaWRvbmcgQ2hlbiIgPGNmbWVfYWRtaW5AMTYzLmNvbT4gd3JvdGU6Cj5TaWdu
ZWQtb2ZmLWJ5OiBMaWRvbmcgQ2hlbiA8Y2ZtZV9hZG1pbkAxNjMuY29tPgo+LS0tCj4ga2VybmVs
L3NjaGVkL2NvcmUuYyB8IDI1ICsrKystLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiAxIGZpbGUgY2hh
bmdlZCwgNCBpbnNlcnRpb25zKCspLCAyMSBkZWxldGlvbnMoLSkKPgo+ZGlmZiAtLWdpdCBhL2tl
cm5lbC9zY2hlZC9jb3JlLmMgYi9rZXJuZWwvc2NoZWQvY29yZS5jCj5pbmRleCBiODAzZTFiLi40
YTVhOTY0IDEwMDY0NAo+LS0tIGEva2VybmVsL3NjaGVkL2NvcmUuYwo+KysrIGIva2VybmVsL3Nj
aGVkL2NvcmUuYwo+QEAgLTIzMTgsMTUgKzIzMTgsMTIgQEAgdm9pZCB3YWtlX3VwX25ld190YXNr
KHN0cnVjdCB0YXNrX3N0cnVjdCAqcCkKPiAKPiAjaWZkZWYgQ09ORklHX1BSRUVNUFRfTk9USUZJ
RVJTCj4gCj4tc3RhdGljIHN0cnVjdCBzdGF0aWNfa2V5IHByZWVtcHRfbm90aWZpZXJfa2V5ID0g
U1RBVElDX0tFWV9JTklUX0ZBTFNFOwo+LQo+IC8qKgo+ICAqIHByZWVtcHRfbm90aWZpZXJfcmVn
aXN0ZXIgLSB0ZWxsIG1lIHdoZW4gY3VycmVudCBpcyBiZWluZyBwcmVlbXB0ZWQgJiByZXNjaGVk
dWxlZAo+ICAqIEBub3RpZmllcjogbm90aWZpZXIgc3RydWN0IHRvIHJlZ2lzdGVyCj4gICovCj4g
dm9pZCBwcmVlbXB0X25vdGlmaWVyX3JlZ2lzdGVyKHN0cnVjdCBwcmVlbXB0X25vdGlmaWVyICpu
b3RpZmllcikKPiB7Cj4tCXN0YXRpY19rZXlfc2xvd19pbmMoJnByZWVtcHRfbm90aWZpZXJfa2V5
KTsKPiAJaGxpc3RfYWRkX2hlYWQoJm5vdGlmaWVyLT5saW5rLCAmY3VycmVudC0+cHJlZW1wdF9u
b3RpZmllcnMpOwo+IH0KPiBFWFBPUlRfU1lNQk9MX0dQTChwcmVlbXB0X25vdGlmaWVyX3JlZ2lz
dGVyKTsKPkBAIC0yMzQwLDExICsyMzM3LDEwIEBAIEVYUE9SVF9TWU1CT0xfR1BMKHByZWVtcHRf
bm90aWZpZXJfcmVnaXN0ZXIpOwo+IHZvaWQgcHJlZW1wdF9ub3RpZmllcl91bnJlZ2lzdGVyKHN0
cnVjdCBwcmVlbXB0X25vdGlmaWVyICpub3RpZmllcikKPiB7Cj4gCWhsaXN0X2RlbCgmbm90aWZp
ZXItPmxpbmspOwo+LQlzdGF0aWNfa2V5X3Nsb3dfZGVjKCZwcmVlbXB0X25vdGlmaWVyX2tleSk7
Cj4gfQo+IEVYUE9SVF9TWU1CT0xfR1BMKHByZWVtcHRfbm90aWZpZXJfdW5yZWdpc3Rlcik7Cj4g
Cj4tc3RhdGljIHZvaWQgX19maXJlX3NjaGVkX2luX3ByZWVtcHRfbm90aWZpZXJzKHN0cnVjdCB0
YXNrX3N0cnVjdCAqY3VycikKPitzdGF0aWMgdm9pZCBmaXJlX3NjaGVkX2luX3ByZWVtcHRfbm90
aWZpZXJzKHN0cnVjdCB0YXNrX3N0cnVjdCAqY3VycikKPiB7Cj4gCXN0cnVjdCBwcmVlbXB0X25v
dGlmaWVyICpub3RpZmllcjsKPiAKPkBAIC0yMzUyLDE0ICsyMzQ4LDkgQEAgc3RhdGljIHZvaWQg
X19maXJlX3NjaGVkX2luX3ByZWVtcHRfbm90aWZpZXJzKHN0cnVjdCB0YXNrX3N0cnVjdCAqY3Vy
cikKPiAJCW5vdGlmaWVyLT5vcHMtPnNjaGVkX2luKG5vdGlmaWVyLCByYXdfc21wX3Byb2Nlc3Nv
cl9pZCgpKTsKPiB9Cj4gCj4tc3RhdGljIF9fYWx3YXlzX2lubGluZSB2b2lkIGZpcmVfc2NoZWRf
aW5fcHJlZW1wdF9ub3RpZmllcnMoc3RydWN0IHRhc2tfc3RydWN0ICpjdXJyKQo+LXsKPi0JaWYg
KHN0YXRpY19rZXlfZmFsc2UoJnByZWVtcHRfbm90aWZpZXJfa2V5KSkKPi0JCV9fZmlyZV9zY2hl
ZF9pbl9wcmVlbXB0X25vdGlmaWVycyhjdXJyKTsKPi19Cj4gCj4gc3RhdGljIHZvaWQKPi1fX2Zp
cmVfc2NoZWRfb3V0X3ByZWVtcHRfbm90aWZpZXJzKHN0cnVjdCB0YXNrX3N0cnVjdCAqY3VyciwK
PitmaXJlX3NjaGVkX291dF9wcmVlbXB0X25vdGlmaWVycyhzdHJ1Y3QgdGFza19zdHJ1Y3QgKmN1
cnIsCj4gCQkJCSAgIHN0cnVjdCB0YXNrX3N0cnVjdCAqbmV4dCkKPiB7Cj4gCXN0cnVjdCBwcmVl
bXB0X25vdGlmaWVyICpub3RpZmllcjsKPkBAIC0yMzY4LDIxICsyMzU5LDEzIEBAIF9fZmlyZV9z
Y2hlZF9vdXRfcHJlZW1wdF9ub3RpZmllcnMoc3RydWN0IHRhc2tfc3RydWN0ICpjdXJyLAo+IAkJ
bm90aWZpZXItPm9wcy0+c2NoZWRfb3V0KG5vdGlmaWVyLCBuZXh0KTsKPiB9Cj4gCj4tc3RhdGlj
IF9fYWx3YXlzX2lubGluZSB2b2lkCj4tZmlyZV9zY2hlZF9vdXRfcHJlZW1wdF9ub3RpZmllcnMo
c3RydWN0IHRhc2tfc3RydWN0ICpjdXJyLAo+LQkJCQkgc3RydWN0IHRhc2tfc3RydWN0ICpuZXh0
KQo+LXsKPi0JaWYgKHN0YXRpY19rZXlfZmFsc2UoJnByZWVtcHRfbm90aWZpZXJfa2V5KSkKPi0J
CV9fZmlyZV9zY2hlZF9vdXRfcHJlZW1wdF9ub3RpZmllcnMoY3VyciwgbmV4dCk7Cj4tfQo+LQo+
ICNlbHNlIC8qICFDT05GSUdfUFJFRU1QVF9OT1RJRklFUlMgKi8KPiAKPi1zdGF0aWMgaW5saW5l
IHZvaWQgZmlyZV9zY2hlZF9pbl9wcmVlbXB0X25vdGlmaWVycyhzdHJ1Y3QgdGFza19zdHJ1Y3Qg
KmN1cnIpCj4rc3RhdGljIHZvaWQgZmlyZV9zY2hlZF9pbl9wcmVlbXB0X25vdGlmaWVycyhzdHJ1
Y3QgdGFza19zdHJ1Y3QgKmN1cnIpCj4gewo+IH0KPiAKPi1zdGF0aWMgaW5saW5lIHZvaWQKPitz
dGF0aWMgdm9pZAo+IGZpcmVfc2NoZWRfb3V0X3ByZWVtcHRfbm90aWZpZXJzKHN0cnVjdCB0YXNr
X3N0cnVjdCAqY3VyciwKPiAJCQkJIHN0cnVjdCB0YXNrX3N0cnVjdCAqbmV4dCkKPiB7Cj4tLSAK
PjIuMS4wCj4K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1175030

FromIngo Molnar <mingo@kernel.org>
Date2015-07-01 09:30 +0200
Message-ID<pHkD0-27B-5@gated-at.bofh.it>
In reply to#1175027
* cfme_admin <cfme_admin@163.com> wrote:

> more information about the bug is below:
> https://bugzilla.kernel.org/show_bug.cgi?id=100701
> 
> the preempt_notifier_unregister function is run in_atomic context, but static_key_slow_dec function may cause sleep.

This bug should already be fixed by this pending commit:

   6efde1d3716b ("sched/preempt, kvm: Fix KVM preempt_notifier usage")

Attached below.

Thanks,

	Ingo

========================>
From 6efde1d3716b7d055d3310bccba2df244cf430d7 Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <peterz@infradead.org>
Date: Thu, 25 Jun 2015 14:55:14 +0200
Subject: [PATCH] sched/preempt, kvm: Fix KVM preempt_notifier usage

The preempt-notifier API needs to sleep with the addition of the
static_key, we do however need to hold off preemption while
modifying the preempt notifier list, otherwise a preemption
could observe an inconsistent list state.

There is no reason to have preemption disabled in the caller,
registering a preempt notifier is a purely task local affair.

Therefore move the preempt_disable into the functions and change
the callers to a preemptible context.

Reported-and-Tested-by: Pontus Fuchs <pontus.fuchs@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Pontus Fuchs <pontus.fuchs@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
Link: http://lkml.kernel.org/r/20150625125514.GA3644@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c | 11 +++++++++++
 virt/kvm/kvm_main.c |  5 +++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c86935a7f1f8..5bcf926e62a4 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2138,7 +2138,12 @@ static struct static_key preempt_notifier_key = STATIC_KEY_INIT_FALSE;
 void preempt_notifier_register(struct preempt_notifier *notifier)
 {
 	static_key_slow_inc(&preempt_notifier_key);
+	/*
+	 * Avoid preemption while changing the preempt notifier list.
+	 */
+	preempt_disable();
 	hlist_add_head(&notifier->link, &current->preempt_notifiers);
+	preempt_enable();
 }
 EXPORT_SYMBOL_GPL(preempt_notifier_register);
 
@@ -2150,7 +2155,13 @@ EXPORT_SYMBOL_GPL(preempt_notifier_register);
  */
 void preempt_notifier_unregister(struct preempt_notifier *notifier)
 {
+	/*
+	 * Avoid preemption while changing the preempt notifier list.
+	 */
+	preempt_disable();
 	hlist_del(&notifier->link);
+	preempt_enable();
+
 	static_key_slow_dec(&preempt_notifier_key);
 }
 EXPORT_SYMBOL_GPL(preempt_notifier_unregister);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 90977418aeb6..d7aafa0458a0 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -128,8 +128,9 @@ int vcpu_load(struct kvm_vcpu *vcpu)
 
 	if (mutex_lock_killable(&vcpu->mutex))
 		return -EINTR;
-	cpu = get_cpu();
 	preempt_notifier_register(&vcpu->preempt_notifier);
+
+	cpu = get_cpu();
 	kvm_arch_vcpu_load(vcpu, cpu);
 	put_cpu();
 	return 0;
@@ -139,8 +140,8 @@ void vcpu_put(struct kvm_vcpu *vcpu)
 {
 	preempt_disable();
 	kvm_arch_vcpu_put(vcpu);
-	preempt_notifier_unregister(&vcpu->preempt_notifier);
 	preempt_enable();
+	preempt_notifier_unregister(&vcpu->preempt_notifier);
 	mutex_unlock(&vcpu->mutex);
 }
 


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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web