Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1176569 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2015-07-03 15:20 +0200 |
| Last post | 2015-07-03 18:00 +0200 |
| Articles | 7 — 3 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.
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Peter Zijlstra <peterz@infradead.org> - 2015-07-03 15:20 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Peter Zijlstra <peterz@infradead.org> - 2015-07-03 17:20 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Paolo Bonzini <pbonzini@redhat.com> - 2015-07-03 17:30 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Paolo Bonzini <pbonzini@redhat.com> - 2015-07-03 17:40 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Peter Zijlstra <peterz@infradead.org> - 2015-07-03 17:50 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Paolo Bonzini <pbonzini@redhat.com> - 2015-07-03 17:50 +0200
Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage Takashi Iwai <tiwai@suse.de> - 2015-07-03 18:00 +0200
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-07-03 15:20 +0200 |
| Subject | Re: [PATCH] sched,kvm: Fix KVM preempt_notifier usage |
| Message-ID | <pI92O-xt-15@gated-at.bofh.it> |
On Fri, Jul 03, 2015 at 02:31:25PM +0200, Paolo Bonzini wrote:
> On 03/07/2015 14:19, Peter Zijlstra wrote:
> > On Fri, Jul 03, 2015 at 01:12:11PM +0200, Paolo Bonzini wrote:
> >> In fact you shouldn't have just tested the patch on a case _without_
> >> preemption notifiers, you should have also benchmarked the impact that
> >> static keys have _with_ preemption notifiers. In a
> >> not-really-artificial case (one single-processor guest running on the
> >> host), the static key patch adds a static_key_slow_inc on a relatively
> >> hot path for KVM, which is not acceptable.
> >
> > Spawning the first vcpu is a hot path?
>
> This is not *spawning* the first VCPU. Basically any critical section
> for vcpu->mutex includes a preempt_notifier_register/unregister pair:
>
> /*
> * Switches to specified vcpu, until a matching vcpu_put()
> */
> int vcpu_load(struct kvm_vcpu *vcpu)
> {
> int cpu;
>
> if (mutex_lock_killable(&vcpu->mutex))
> return -EINTR;
> cpu = get_cpu();
> preempt_notifier_register(&vcpu->preempt_notifier);
> kvm_arch_vcpu_load(vcpu, cpu);
> put_cpu();
> return 0;
> }
>
> void vcpu_put(struct kvm_vcpu *vcpu)
> {
> preempt_disable();
> kvm_arch_vcpu_put(vcpu);
> preempt_notifier_unregister(&vcpu->preempt_notifier);
> preempt_enable();
> mutex_unlock(&vcpu->mutex);
> }
>
> So basically you're adding at least one static_key_slow_inc/dec pair to
> every userspace exit.
Ugh, ok that is not what I was expecting to happen. I'll ask Ingo to
queue a revert until we can fix this better.
I thought these were vcpu create/destroy functions.
That said, the slow_inc/dec are really only slow on the 0<->!0
transitions.
But, could we rework the code so that you register the preempt notifier
when creating the vcpu thread and leave it installed forevermore?
--
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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-07-03 17:20 +0200 |
| Message-ID | <pIaUW-1GA-15@gated-at.bofh.it> |
| In reply to | #1176569 |
On Fri, Jul 03, 2015 at 03:17:13PM +0200, Peter Zijlstra wrote:
> But, could we rework the code so that you register the preempt notifier
> when creating the vcpu thread and leave it installed forevermore?
OK, it looks like there is no fixed relation between a thread and a vcpu
:/
Would something like the below (on top of all the others) work for you?
---
include/linux/preempt.h | 2 ++
kernel/sched/core.c | 18 +++++++++++++++---
virt/kvm/kvm_main.c | 7 +++++--
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 0f1534acaf60..84991f185173 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -293,6 +293,8 @@ struct preempt_notifier {
struct preempt_ops *ops;
};
+void preempt_notifier_inc(void);
+void preempt_notifier_dec(void);
void preempt_notifier_register(struct preempt_notifier *notifier);
void preempt_notifier_unregister(struct preempt_notifier *notifier);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 6169c167ac98..5ddbcb720fc6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2320,13 +2320,27 @@ void wake_up_new_task(struct task_struct *p)
static struct static_key preempt_notifier_key = STATIC_KEY_INIT_FALSE;
+void preempt_notifier_inc(void)
+{
+ static_key_slow_inc(&preempt_notifier_key);
+}
+EXPORT_SYMBOL_GPL(preempt_notifier_inc);
+
+void preempt_notifier_dec(void)
+{
+ static_key_slow_dec(&preempt_notifier_key);
+}
+EXPORT_SYMBOL_GPL(preempt_notifier_dec);
+
/**
* preempt_notifier_register - tell me when current is being preempted & rescheduled
* @notifier: notifier struct to register
*/
void preempt_notifier_register(struct preempt_notifier *notifier)
{
- static_key_slow_inc(&preempt_notifier_key);
+ if (!static_key_false(&preempt_notifier_key))
+ WARN(1, "registering preempt_notifier while notifiers disabled\n");
+
/*
* Avoid preemption while changing the preempt notifier list.
*/
@@ -2350,8 +2364,6 @@ void preempt_notifier_unregister(struct preempt_notifier *notifier)
preempt_disable();
hlist_del(¬ifier->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 d7aafa0458a0..8136be28d76c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -128,9 +128,9 @@ int vcpu_load(struct kvm_vcpu *vcpu)
if (mutex_lock_killable(&vcpu->mutex))
return -EINTR;
- preempt_notifier_register(&vcpu->preempt_notifier);
cpu = get_cpu();
+ preempt_notifier_register(&vcpu->preempt_notifier);
kvm_arch_vcpu_load(vcpu, cpu);
put_cpu();
return 0;
@@ -140,8 +140,8 @@ void vcpu_put(struct kvm_vcpu *vcpu)
{
preempt_disable();
kvm_arch_vcpu_put(vcpu);
- preempt_enable();
preempt_notifier_unregister(&vcpu->preempt_notifier);
+ preempt_enable();
mutex_unlock(&vcpu->mutex);
}
@@ -513,6 +513,8 @@ static struct kvm *kvm_create_vm(unsigned long type)
list_add(&kvm->vm_list, &vm_list);
spin_unlock(&kvm_lock);
+ preempt_notifier_inc();
+
return kvm;
out_err:
@@ -612,6 +614,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
cleanup_srcu_struct(&kvm->irq_srcu);
cleanup_srcu_struct(&kvm->srcu);
kvm_arch_free_vm(kvm);
+ preempt_notifier_dec();
hardware_disable_all();
mmdrop(mm);
}
--
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] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2015-07-03 17:30 +0200 |
| Message-ID | <pIb4D-1JT-29@gated-at.bofh.it> |
| In reply to | #1176653 |
On 03/07/2015 17:16, Peter Zijlstra wrote:
> On Fri, Jul 03, 2015 at 03:17:13PM +0200, Peter Zijlstra wrote:
>> But, could we rework the code so that you register the preempt notifier
>> when creating the vcpu thread and leave it installed forevermore?
>
> OK, it looks like there is no fixed relation between a thread and a vcpu
> :/
>
> Would something like the below (on top of all the others) work for you?
This looks fine, but one would do the same thing without the previous
patch, i.e. directly on top of Linus's master---right? The patch in tip
is a red herring, and the hunks below are reverting large parts of it.
I'm going to send a pull request to Linus anyway, either tonight or
tomorrow morning. Send it with SoB, so that it applies to Linus's
master, and I can include it. This way bisection is also better preserved.
Paolo
> ---
> include/linux/preempt.h | 2 ++
> kernel/sched/core.c | 18 +++++++++++++++---
> virt/kvm/kvm_main.c | 7 +++++--
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/preempt.h b/include/linux/preempt.h
> index 0f1534acaf60..84991f185173 100644
> --- a/include/linux/preempt.h
> +++ b/include/linux/preempt.h
> @@ -293,6 +293,8 @@ struct preempt_notifier {
> struct preempt_ops *ops;
> };
>
> +void preempt_notifier_inc(void);
> +void preempt_notifier_dec(void);
> void preempt_notifier_register(struct preempt_notifier *notifier);
> void preempt_notifier_unregister(struct preempt_notifier *notifier);
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 6169c167ac98..5ddbcb720fc6 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2320,13 +2320,27 @@ void wake_up_new_task(struct task_struct *p)
>
> static struct static_key preempt_notifier_key = STATIC_KEY_INIT_FALSE;
>
> +void preempt_notifier_inc(void)
> +{
> + static_key_slow_inc(&preempt_notifier_key);
> +}
> +EXPORT_SYMBOL_GPL(preempt_notifier_inc);
> +
> +void preempt_notifier_dec(void)
> +{
> + static_key_slow_dec(&preempt_notifier_key);
> +}
> +EXPORT_SYMBOL_GPL(preempt_notifier_dec);
> +
> /**
> * preempt_notifier_register - tell me when current is being preempted & rescheduled
> * @notifier: notifier struct to register
> */
> void preempt_notifier_register(struct preempt_notifier *notifier)
> {
> - static_key_slow_inc(&preempt_notifier_key);
> + if (!static_key_false(&preempt_notifier_key))
> + WARN(1, "registering preempt_notifier while notifiers disabled\n");
> +
> /*
> * Avoid preemption while changing the preempt notifier list.
> */
> @@ -2350,8 +2364,6 @@ void preempt_notifier_unregister(struct preempt_notifier *notifier)
> preempt_disable();
> hlist_del(¬ifier->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 d7aafa0458a0..8136be28d76c 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -128,9 +128,9 @@ int vcpu_load(struct kvm_vcpu *vcpu)
>
> if (mutex_lock_killable(&vcpu->mutex))
> return -EINTR;
> - preempt_notifier_register(&vcpu->preempt_notifier);
>
> cpu = get_cpu();
> + preempt_notifier_register(&vcpu->preempt_notifier);
> kvm_arch_vcpu_load(vcpu, cpu);
> put_cpu();
> return 0;
> @@ -140,8 +140,8 @@ void vcpu_put(struct kvm_vcpu *vcpu)
> {
> preempt_disable();
> kvm_arch_vcpu_put(vcpu);
> - preempt_enable();
> preempt_notifier_unregister(&vcpu->preempt_notifier);
> + preempt_enable();
> mutex_unlock(&vcpu->mutex);
> }
>
> @@ -513,6 +513,8 @@ static struct kvm *kvm_create_vm(unsigned long type)
> list_add(&kvm->vm_list, &vm_list);
> spin_unlock(&kvm_lock);
>
> + preempt_notifier_inc();
> +
> return kvm;
>
> out_err:
> @@ -612,6 +614,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
> cleanup_srcu_struct(&kvm->irq_srcu);
> cleanup_srcu_struct(&kvm->srcu);
> kvm_arch_free_vm(kvm);
> + preempt_notifier_dec();
> hardware_disable_all();
> mmdrop(mm);
> }
>
--
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] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2015-07-03 17:40 +0200 |
| Message-ID | <pIbei-1No-29@gated-at.bofh.it> |
| In reply to | #1176668 |
On 03/07/2015 17:26, Paolo Bonzini wrote:
>
>
> On 03/07/2015 17:16, Peter Zijlstra wrote:
>> On Fri, Jul 03, 2015 at 03:17:13PM +0200, Peter Zijlstra wrote:
>>> But, could we rework the code so that you register the preempt notifier
>>> when creating the vcpu thread and leave it installed forevermore?
>>
>> OK, it looks like there is no fixed relation between a thread and a vcpu
>> :/
>>
>> Would something like the below (on top of all the others) work for you?
>
> This looks fine, but one would do the same thing without the previous
> patch, i.e. directly on top of Linus's master---right? The patch in tip
> is a red herring, and the hunks below are reverting large parts of it.
So basically this. Can you reply with SoB and maybe Acked-by?
------------- 8< ---------------
From: Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH] sched, preempt_notifier: separate notifier registration from static_key inc/dec
Commit 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
had two problems. First, 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. KVM correctly registers and
unregisters preempt notifiers with preemption disabled, so the sleep
caused dmesg splats.
Second, KVM registers and unregisters preemption notifiers very often
(in vcpu_load/vcpu_put). With a single uniprocessor guest the static key
would move between 0 and 1 continuously, hitting the slow path on every
userspace exit.
To fix this, wrap the static_key inc/dec in a new API, and call it from
KVM.
Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
Reported-by: Pontus Fuchs <pontus.fuchs@gmail.com>
Reported-by: Takashi Iwai <tiwai@suse.de>
Not-signed-off-by: Peter Zijlstra <peterz@infradead.org>
Not-acked-by: Peter Zijlstra <peterz@infradead.org>
Not-signed-off-by: Paolo Bonzini <pbonzini@redhat.com
diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 0f1534acaf60..84991f185173 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -293,6 +293,8 @@ struct preempt_notifier {
struct preempt_ops *ops;
};
+void preempt_notifier_inc(void);
+void preempt_notifier_dec(void);
void preempt_notifier_register(struct preempt_notifier *notifier);
void preempt_notifier_unregister(struct preempt_notifier *notifier);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b803e1b8ab0c..552710ab19e0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2320,13 +2320,27 @@ void wake_up_new_task(struct task_struct *p)
static struct static_key preempt_notifier_key = STATIC_KEY_INIT_FALSE;
+void preempt_notifier_inc(void)
+{
+ static_key_slow_inc(&preempt_notifier_key);
+}
+EXPORT_SYMBOL_GPL(preempt_notifier_inc);
+
+void preempt_notifier_dec(void)
+{
+ static_key_slow_dec(&preempt_notifier_key);
+}
+EXPORT_SYMBOL_GPL(preempt_notifier_dec);
+
/**
* preempt_notifier_register - tell me when current is being preempted & rescheduled
* @notifier: notifier struct to register
*/
void preempt_notifier_register(struct preempt_notifier *notifier)
{
- static_key_slow_inc(&preempt_notifier_key);
+ if (!static_key_false(&preempt_notifier_key))
+ WARN(1, "registering preempt_notifier while notifiers disabled\n");
+
hlist_add_head(¬ifier->link, ¤t->preempt_notifiers);
}
EXPORT_SYMBOL_GPL(preempt_notifier_register);
@@ -2340,7 +2354,6 @@ EXPORT_SYMBOL_GPL(preempt_notifier_register);
void preempt_notifier_unregister(struct preempt_notifier *notifier)
{
hlist_del(¬ifier->link);
- 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 848af90b8091..8b8a44453670 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -553,6 +553,8 @@ static struct kvm *kvm_create_vm(unsigned long type)
list_add(&kvm->vm_list, &vm_list);
spin_unlock(&kvm_lock);
+ preempt_notifier_inc();
+
return kvm;
out_err:
@@ -620,6 +622,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
cleanup_srcu_struct(&kvm->irq_srcu);
cleanup_srcu_struct(&kvm->srcu);
kvm_arch_free_vm(kvm);
+ preempt_notifier_dec();
hardware_disable_all();
mmdrop(mm);
}
--
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] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-07-03 17:50 +0200 |
| Message-ID | <pIbnX-1T7-15@gated-at.bofh.it> |
| In reply to | #1176679 |
On Fri, Jul 03, 2015 at 05:38:42PM +0200, Paolo Bonzini wrote:
> So basically this. Can you reply with SoB and maybe Acked-by?
Ah, thanks for doing that!
> ------------- 8< ---------------
> From: Peter Zijlstra <peterz@infradead.org>
> Subject: [PATCH] sched, preempt_notifier: separate notifier registration from static_key inc/dec
>
> Commit 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
> had two problems. First, 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. KVM correctly registers and
> unregisters preempt notifiers with preemption disabled, so the sleep
> caused dmesg splats.
>
> Second, KVM registers and unregisters preemption notifiers very often
> (in vcpu_load/vcpu_put). With a single uniprocessor guest the static key
> would move between 0 and 1 continuously, hitting the slow path on every
> userspace exit.
>
> To fix this, wrap the static_key inc/dec in a new API, and call it from
> KVM.
>
> Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
> Reported-by: Pontus Fuchs <pontus.fuchs@gmail.com>
> Reported-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
--
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] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2015-07-03 17:50 +0200 |
| Message-ID | <pIbnX-1T7-17@gated-at.bofh.it> |
| In reply to | #1176682 |
On 03/07/2015 17:42, Peter Zijlstra wrote:
> On Fri, Jul 03, 2015 at 05:38:42PM +0200, Paolo Bonzini wrote:
>> So basically this. Can you reply with SoB and maybe Acked-by?
>
> Ah, thanks for doing that!
>
>> ------------- 8< ---------------
>> From: Peter Zijlstra <peterz@infradead.org>
>> Subject: [PATCH] sched, preempt_notifier: separate notifier registration from static_key inc/dec
>>
>> Commit 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
>> had two problems. First, 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. KVM correctly registers and
>> unregisters preempt notifiers with preemption disabled, so the sleep
>> caused dmesg splats.
>>
>> Second, KVM registers and unregisters preemption notifiers very often
>> (in vcpu_load/vcpu_put). With a single uniprocessor guest the static key
>> would move between 0 and 1 continuously, hitting the slow path on every
>> userspace exit.
>>
>> To fix this, wrap the static_key inc/dec in a new API, and call it from
>> KVM.
>>
>> Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
>> Reported-by: Pontus Fuchs <pontus.fuchs@gmail.com>
>> Reported-by: Takashi Iwai <tiwai@suse.de>
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Ok, I'm crossing fingers and including this in my pull request in order
to preserve bisectability. Thanks.
Paolo
--
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] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2015-07-03 18:00 +0200 |
| Message-ID | <pIbxD-1Wv-3@gated-at.bofh.it> |
| In reply to | #1176683 |
At Fri, 3 Jul 2015 17:46:06 +0200,
Paolo Bonzini wrote:
>
>
>
> On 03/07/2015 17:42, Peter Zijlstra wrote:
> > On Fri, Jul 03, 2015 at 05:38:42PM +0200, Paolo Bonzini wrote:
> >> So basically this. Can you reply with SoB and maybe Acked-by?
> >
> > Ah, thanks for doing that!
> >
> >> ------------- 8< ---------------
> >> From: Peter Zijlstra <peterz@infradead.org>
> >> Subject: [PATCH] sched, preempt_notifier: separate notifier registration from static_key inc/dec
> >>
> >> Commit 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
> >> had two problems. First, 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. KVM correctly registers and
> >> unregisters preempt notifiers with preemption disabled, so the sleep
> >> caused dmesg splats.
> >>
> >> Second, KVM registers and unregisters preemption notifiers very often
> >> (in vcpu_load/vcpu_put). With a single uniprocessor guest the static key
> >> would move between 0 and 1 continuously, hitting the slow path on every
> >> userspace exit.
> >>
> >> To fix this, wrap the static_key inc/dec in a new API, and call it from
> >> KVM.
> >>
> >> Fixes: 1cde2930e154 ("sched/preempt: Add static_key() to preempt_notifiers")
> >> Reported-by: Pontus Fuchs <pontus.fuchs@gmail.com>
> >> Reported-by: Takashi Iwai <tiwai@suse.de>
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Ok, I'm crossing fingers and including this in my pull request in order
> to preserve bisectability. Thanks.
I checked the patch now and confirmed that it fixes the regressions
(the warnings and the sluggish mouse pointer). Feel free to my
tested-by tag, if any.
Tested-by: Takashi Iwai <tiwai@suse.de>
Thanks!
Takashi
--
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