Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/18/2012 12:27 AM, Raghavendra K T wrote: On 01/17/2012 04:32 PM, Marcelo Tosatti wrote: On Sat, Jan 14, 2012 at 11:56:46PM +0530, Raghavendra K T wrote: [...] + || (vcpu->requests& ~(1ULarch.apf.halted) || !list_empty_careful(&vcpu->async_pf.done) || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED + || kvm_check_request(KVM_REQ_PVLOCK_KICK, vcpu) The bit should only be read here (kvm_arch_vcpu_runnable), but cleared on vcpu entry (along with the other kvm_check_request processing). [...] I had tried alternative approach earlier, I think that is closer to your expectation. where - flag is read in kvm_arch_vcpu_runnable - flag cleared in vcpu entry along with others. But it needs per vcpu flag to remember that pv_unhalted while clearing the flag in vcpu enter [ patch below ]. Could not find third alternative though. [...] do you think having pv_unhalt flag in below patch cause problem to live migration still? (vcpu->request bit is retained as is) OR do we have to have KVM_GET_MP_STATE changes also with below patch you mentioned earlier. Avi, Marcello, Please let me know, any comments you have on how should it look like in next version? Should I get rid of KVM_REQ_PVLOCK_KICK bit in vcpu->request and have only pv_unahlt flag as below and also add MSR as suggested? ---8<--- diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c38efd7..1bf8fa8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5684,6 +5717,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) r = 1; goto out; } + if (kvm_check_request(KVM_REQ_PVKICK, vcpu)) { + vcpu->pv_unhalted = 1; + r = 1; + goto out; + } if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) record_steal_time(vcpu); if (kvm_check_request(KVM_REQ_NMI, vcpu)) @@ -6683,6 +6720,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) !vcpu->arch.apf.halted) || !list_empty_careful(&vcpu->async_pf.done) || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED + || (kvm_test_request(KVM_REQ_PVKICK, vcpu) || vcpu->pv_unhalted) || atomic_read(&vcpu->arch.nmi_queued) || (kvm_arch_interrupt_allowed(vcpu) && kvm_cpu_has_interrupt(vcpu)); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index d526231..a48e0f2 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -154,6 +155,8 @@ struct kvm_vcpu { #endif struct kvm_vcpu_arch arch; + + int pv_unhalted; }; static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu) @@ -770,5 +773,12 @@ static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu) } } +static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu) +{ + if (test_bit(req, &vcpu->requests)) + return true; + else + return false; +} #endif diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d9cfb78..55c44a2 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -226,6 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id) vcpu->kvm = kvm; vcpu->vcpu_id = id; vcpu->pid = NULL; + vcpu->pv_unhalted = 0; init_waitqueue_head(&vcpu->wq); kvm_async_pf_vcpu_init(vcpu); @@ -1509,11 +1510,12 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn) void kvm_vcpu_block(struct kvm_vcpu *vcpu) { DEFINE_WAIT(wait); for (;;) { prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); if (kvm_arch_vcpu_runnable(vcpu)) { + vcpu->pv_unhalted = 0; kvm_make_request(KVM_REQ_UNHALT, vcpu); break; } ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/24/2012 07:38 PM, Avi Kivity wrote: On 01/18/2012 11:52 PM, Jeremy Fitzhardinge wrote: On 01/19/2012 12:54 AM, Srivatsa Vaddagiri wrote: That logic relies on the "kick" being level triggered, so that "kick" before "block" will cause the block to fall out immediately. If you're using "hlt" as the block and it has the usual edge-triggered behaviour, what stops a "kick-before-hlt" from losing the kick? Hmm ..'hlt' should result in a check for kick request (in hypervisor context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick vcpu0 will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should check before it puts vcpu0 to sleep because of trapped 'hlt' instruction. Won't that trap the 'kick-before-hlt' case? What am I missing here? Nothing, that sounds fine. It wasn't clear to me that your kick operation left persistent state, and so has a level-triggered effect on hlt. btw, this persistent state needs to be saved/restored for live migration. Best to put it into some MSR. I did not quite get it. Did you mean, add a new MSR to msrs_to_save[], and may be retain only the kicked/pv_unhalt flag (persistent state) and get rid of PVLOCK_KICK vcpu->request? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/18/2012 11:52 PM, Jeremy Fitzhardinge wrote: > On 01/19/2012 12:54 AM, Srivatsa Vaddagiri wrote: > > > >> That logic relies on the "kick" being level triggered, so that "kick" > >> before "block" will cause the block to fall out immediately. If you're > >> using "hlt" as the block and it has the usual edge-triggered behaviour, > >> what stops a "kick-before-hlt" from losing the kick? > > Hmm ..'hlt' should result in a check for kick request (in hypervisor > > context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick > > vcpu0 > > will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should > > check > > before it puts vcpu0 to sleep because of trapped 'hlt' instruction. > > > > Won't that trap the 'kick-before-hlt' case? What am I missing here? > > Nothing, that sounds fine. It wasn't clear to me that your kick > operation left persistent state, and so has a level-triggered effect on hlt. > btw, this persistent state needs to be saved/restored for live migration. Best to put it into some MSR. -- error compiling committee.c: too many arguments to function ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/19/2012 12:54 AM, Srivatsa Vaddagiri wrote: > >> That logic relies on the "kick" being level triggered, so that "kick" >> before "block" will cause the block to fall out immediately. If you're >> using "hlt" as the block and it has the usual edge-triggered behaviour, >> what stops a "kick-before-hlt" from losing the kick? > Hmm ..'hlt' should result in a check for kick request (in hypervisor > context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick > vcpu0 > will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should check > before it puts vcpu0 to sleep because of trapped 'hlt' instruction. > > Won't that trap the 'kick-before-hlt' case? What am I missing here? Nothing, that sounds fine. It wasn't clear to me that your kick operation left persistent state, and so has a level-triggered effect on hlt. J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
* Jeremy Fitzhardinge [2012-01-18 12:34:42]:
> >> What prevents a kick from being lost here, if say, the waiter is at
> >> local_irq_save in kvm_lock_spinning, before the lock/want assignments?
> > The waiter does check for lock becoming available before actually
> > sleeping:
> >
> > + /*
> > +* check again make sure it didn't become free while
> > +* we weren't looking.
> > +*/
> > + if (ACCESS_ONCE(lock->tickets.head) == want) {
> > + add_stats(TAKEN_SLOW_PICKUP, 1);
> > + goto out;
> > + }
>
> That logic relies on the "kick" being level triggered, so that "kick"
> before "block" will cause the block to fall out immediately. If you're
> using "hlt" as the block and it has the usual edge-triggered behaviour,
> what stops a "kick-before-hlt" from losing the kick?
Hmm ..'hlt' should result in a check for kick request (in hypervisor
context) before vcpu is put to sleep. IOW vcpu1 that is attempting to kick vcpu0
will set a 'somebody_tried_kicking_vcpu0' flag, which hypervisor should check
before it puts vcpu0 to sleep because of trapped 'hlt' instruction.
Won't that trap the 'kick-before-hlt' case? What am I missing here?
- vatsa
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/17/2012 10:33 PM, Srivatsa Vaddagiri wrote:
> * Marcelo Tosatti [2012-01-17 09:02:11]:
>
>>> +/* Kick vcpu waiting on @lock->head to reach value @ticket */
>>> +static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket)
>>> +{
>>> + int cpu;
>>> + int apicid;
>>> +
>>> + add_stats(RELEASED_SLOW, 1);
>>> +
>>> + for_each_cpu(cpu, &waiting_cpus) {
>>> + const struct kvm_lock_waiting *w = &per_cpu(lock_waiting, cpu);
>>> + if (ACCESS_ONCE(w->lock) == lock &&
>>> + ACCESS_ONCE(w->want) == ticket) {
>>> + add_stats(RELEASED_SLOW_KICKED, 1);
>>> + apicid = per_cpu(x86_cpu_to_apicid, cpu);
>>> + kvm_kick_cpu(apicid);
>>> + break;
>>> + }
>>> + }
>> What prevents a kick from being lost here, if say, the waiter is at
>> local_irq_save in kvm_lock_spinning, before the lock/want assignments?
> The waiter does check for lock becoming available before actually
> sleeping:
>
> + /*
> +* check again make sure it didn't become free while
> +* we weren't looking.
> +*/
> + if (ACCESS_ONCE(lock->tickets.head) == want) {
> + add_stats(TAKEN_SLOW_PICKUP, 1);
> + goto out;
> + }
That logic relies on the "kick" being level triggered, so that "kick"
before "block" will cause the block to fall out immediately. If you're
using "hlt" as the block and it has the usual edge-triggered behaviour,
what stops a "kick-before-hlt" from losing the kick?
J
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/17/2012 04:32 PM, Marcelo Tosatti wrote:
On Sat, Jan 14, 2012 at 11:56:46PM +0530, Raghavendra K T wrote:
Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c7b05fc..4d7a950 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5754,8 +5754,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
local_irq_disable();
- if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests
- || need_resched() || signal_pending(current)) {
+ if (vcpu->mode == EXITING_GUEST_MODE
+|| (vcpu->requests& ~(1ULarch.apf.halted)
|| !list_empty_careful(&vcpu->async_pf.done)
|| vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
+ || kvm_check_request(KVM_REQ_PVLOCK_KICK, vcpu)
The bit should only be read here (kvm_arch_vcpu_runnable), but cleared
on vcpu entry (along with the other kvm_check_request processing).
Then the first hunk becomes unnecessary.
true. [ patch below ]
Please do not mix host/guest patches.
yes, will be taken care in next version..
I had tried alternative approach earlier, I think that is closer
to your expectation.
where
- flag is read in kvm_arch_vcpu_runnable
- flag cleared in vcpu entry along with others.
But it needs per vcpu flag to remember that pv_unhalted while clearing
the flag in vcpu enter [ patch below ]. Could not find third alternative
though.
Simply clearing the request bit in vcpu entry had made guest hang in
*rare* scenario. [as kick will be lost].
[ I had observed guest hang after 4 iteration of kernbench with 1:3
overcommit. with 2/3 guest running while 1 hogs ]
Avi,
do you think having pv_unhalt flag in below patch cause problem to
live migration still? (vcpu->request bit is retained as is) OR do we
have to have KVM_GET_MP_STATE changes also with below patch you
mentioned earlier.
---8<---
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c38efd7..1bf8fa8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5684,6 +5717,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
r = 1;
goto out;
}
+ if (kvm_check_request(KVM_REQ_PVKICK, vcpu)) {
+ vcpu->pv_unhalted = 1;
+ r = 1;
+ goto out;
+ }
if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
record_steal_time(vcpu);
if (kvm_check_request(KVM_REQ_NMI, vcpu))
@@ -6683,6 +6720,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
!vcpu->arch.apf.halted)
|| !list_empty_careful(&vcpu->async_pf.done)
|| vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
+ || (kvm_test_request(KVM_REQ_PVKICK, vcpu) || vcpu->pv_unhalted)
|| atomic_read(&vcpu->arch.nmi_queued) ||
(kvm_arch_interrupt_allowed(vcpu) &&
kvm_cpu_has_interrupt(vcpu));
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d526231..a48e0f2 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -154,6 +155,8 @@ struct kvm_vcpu {
#endif
struct kvm_vcpu_arch arch;
+
+ int pv_unhalted;
};
static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
@@ -770,5 +773,12 @@ static inline bool kvm_check_request(int req,
struct kvm_vcpu *vcpu)
}
}
+static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
+{
+ if (test_bit(req, &vcpu->requests))
+ return true;
+ else
+ return false;
+}
#endif
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d9cfb78..55c44a2 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -226,6 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm
*kvm, unsigned id)
vcpu->kvm = kvm;
vcpu->vcpu_id = id;
vcpu->pid = NULL;
+ vcpu->pv_unhalted = 0;
init_waitqueue_head(&vcpu->wq);
kvm_async_pf_vcpu_init(vcpu);
@@ -1509,11 +1510,12 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
void kvm_vcpu_block(struct kvm_vcpu *vcpu)
{
DEFINE_WAIT(wait);
for (;;) {
prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
if (kvm_arch_vcpu_runnable(vcpu)) {
+ vcpu->pv_unhalted = 0;
kvm_make_request(KVM_REQ_UNHALT, vcpu);
break;
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
* Marcelo Tosatti [2012-01-17 09:02:11]:
> > +/* Kick vcpu waiting on @lock->head to reach value @ticket */
> > +static void kvm_unlock_kick(struct arch_spinlock *lock, __ticket_t ticket)
> > +{
> > + int cpu;
> > + int apicid;
> > +
> > + add_stats(RELEASED_SLOW, 1);
> > +
> > + for_each_cpu(cpu, &waiting_cpus) {
> > + const struct kvm_lock_waiting *w = &per_cpu(lock_waiting, cpu);
> > + if (ACCESS_ONCE(w->lock) == lock &&
> > + ACCESS_ONCE(w->want) == ticket) {
> > + add_stats(RELEASED_SLOW_KICKED, 1);
> > + apicid = per_cpu(x86_cpu_to_apicid, cpu);
> > + kvm_kick_cpu(apicid);
> > + break;
> > + }
> > + }
>
> What prevents a kick from being lost here, if say, the waiter is at
> local_irq_save in kvm_lock_spinning, before the lock/want assignments?
The waiter does check for lock becoming available before actually
sleeping:
+ /*
+* check again make sure it didn't become free while
+* we weren't looking.
+*/
+ if (ACCESS_ONCE(lock->tickets.head) == want) {
+ add_stats(TAKEN_SLOW_PICKUP, 1);
+ goto out;
+ }
- vatsa
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On Sat, Jan 14, 2012 at 11:56:46PM +0530, Raghavendra K T wrote:
> Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
>
> During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has
> required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so,
> support for pv-ticketlocks is registered via pv_lock_ops.
>
> Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu.
>
> Signed-off-by: Srivatsa Vaddagiri
> Signed-off-by: Suzuki Poulose
> Signed-off-by: Raghavendra K T
> ---
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 7a94987..cf5327c 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -195,10 +195,20 @@ void kvm_async_pf_task_wait(u32 token);
> void kvm_async_pf_task_wake(u32 token);
> u32 kvm_read_and_reset_pf_reason(void);
> extern void kvm_disable_steal_time(void);
> -#else
> -#define kvm_guest_init() do { } while (0)
> +
> +#ifdef CONFIG_PARAVIRT_SPINLOCKS
> +void __init kvm_spinlock_init(void);
> +#else /* CONFIG_PARAVIRT_SPINLOCKS */
> +static void kvm_spinlock_init(void)
> +{
> +}
> +#endif /* CONFIG_PARAVIRT_SPINLOCKS */
> +
> +#else /* CONFIG_KVM_GUEST */
> +#define kvm_guest_init() do {} while (0)
> #define kvm_async_pf_task_wait(T) do {} while(0)
> #define kvm_async_pf_task_wake(T) do {} while(0)
> +
> static inline u32 kvm_read_and_reset_pf_reason(void)
> {
> return 0;
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index a9c2116..ec55a0b 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -33,6 +33,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -545,6 +546,7 @@ static void __init kvm_smp_prepare_boot_cpu(void)
> #endif
> kvm_guest_cpu_init();
> native_smp_prepare_boot_cpu();
> + kvm_spinlock_init();
> }
>
> static void __cpuinit kvm_guest_cpu_online(void *dummy)
> @@ -627,3 +629,250 @@ static __init int activate_jump_labels(void)
> return 0;
> }
> arch_initcall(activate_jump_labels);
> +
> +#ifdef CONFIG_PARAVIRT_SPINLOCKS
> +
> +enum kvm_contention_stat {
> + TAKEN_SLOW,
> + TAKEN_SLOW_PICKUP,
> + RELEASED_SLOW,
> + RELEASED_SLOW_KICKED,
> + NR_CONTENTION_STATS
> +};
> +
> +#ifdef CONFIG_KVM_DEBUG_FS
> +
> +static struct kvm_spinlock_stats
> +{
> + u32 contention_stats[NR_CONTENTION_STATS];
> +
> +#define HISTO_BUCKETS30
> + u32 histo_spin_blocked[HISTO_BUCKETS+1];
> +
> + u64 time_blocked;
> +} spinlock_stats;
> +
> +static u8 zero_stats;
> +
> +static inline void check_zero(void)
> +{
> + u8 ret;
> + u8 old = ACCESS_ONCE(zero_stats);
> + if (unlikely(old)) {
> + ret = cmpxchg(&zero_stats, old, 0);
> + /* This ensures only one fellow resets the stat */
> + if (ret == old)
> + memset(&spinlock_stats, 0, sizeof(spinlock_stats));
> + }
> +}
> +
> +static inline void add_stats(enum kvm_contention_stat var, u32 val)
> +{
> + check_zero();
> + spinlock_stats.contention_stats[var] += val;
> +}
> +
> +
> +static inline u64 spin_time_start(void)
> +{
> + return sched_clock();
> +}
> +
> +static void __spin_time_accum(u64 delta, u32 *array)
> +{
> + unsigned index = ilog2(delta);
> +
> + check_zero();
> +
> + if (index < HISTO_BUCKETS)
> + array[index]++;
> + else
> + array[HISTO_BUCKETS]++;
> +}
> +
> +static inline void spin_time_accum_blocked(u64 start)
> +{
> + u32 delta = sched_clock() - start;
> +
> + __spin_time_accum(delta, spinlock_stats.histo_spin_blocked);
> + spinlock_stats.time_blocked += delta;
> +}
> +
> +static struct dentry *d_spin_debug;
> +static struct dentry *d_kvm_debug;
> +
> +struct dentry *kvm_init_debugfs(void)
> +{
> + d_kvm_debug = debugfs_create_dir("kvm", NULL);
> + if (!d_kvm_debug)
> + printk(KERN_WARNING "Could not create 'kvm' debugfs
> directory\n");
> +
> + return d_kvm_debug;
> +}
> +
> +static int __init kvm_spinlock_debugfs(void)
> +{
> + struct dentry *d_kvm = kvm_init_debugfs();
> +
> + if (d_kvm == NULL)
> + return -ENOMEM;
> +
> + d_spin_debug = debugfs_create_dir("spinlocks", d_kvm);
> +
> + debugfs_create_u8("zero_stats", 0644, d_spin_debug, &zero_stats);
> +
> + debugfs_create_u32("taken_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW]);
> + debugfs_create_u32("taken_slow_pickup", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW_PICKUP]);
> +
> + debugfs_create_u32("released_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW]);
> + debugfs_create_u32("released_slow_kicked", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW_KICKED]);
> +
> + debugfs_create_u64("time_blocked", 0444, d_spin
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/17/2012 01:47 AM, Avi Kivity wrote: > On 01/16/2012 04:13 PM, Raghavendra K T wrote: >>> Please drop all of these and replace with tracepoints in the appropriate >>> spots. Everything else (including the historgram) can be reconstructed >>> the tracepoints in userspace. >>> >> >> I think Jeremy pointed that tracepoints use spinlocks and hence debugfs >> is the option.. no ? >> > Yeah, I think you're right. What a pity. Yes, I went through the same thought process. J ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/16/2012 04:13 PM, Raghavendra K T wrote: >> Please drop all of these and replace with tracepoints in the appropriate >> spots. Everything else (including the historgram) can be reconstructed >> the tracepoints in userspace. >> > > > I think Jeremy pointed that tracepoints use spinlocks and hence debugfs > is the option.. no ? > Yeah, I think you're right. What a pity. -- error compiling committee.c: too many arguments to function ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/16/2012 02:35 PM, Avi Kivity wrote:
On 01/14/2012 08:26 PM, Raghavendra K T wrote:
Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has
required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so,
support for pv-ticketlocks is registered via pv_lock_ops.
Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu.
+
+ debugfs_create_u8("zero_stats", 0644, d_spin_debug,&zero_stats);
+
+ debugfs_create_u32("taken_slow", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[TAKEN_SLOW]);
+ debugfs_create_u32("taken_slow_pickup", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[TAKEN_SLOW_PICKUP]);
+
+ debugfs_create_u32("released_slow", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[RELEASED_SLOW]);
+ debugfs_create_u32("released_slow_kicked", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[RELEASED_SLOW_KICKED]);
+
+ debugfs_create_u64("time_blocked", 0444, d_spin_debug,
+ &spinlock_stats.time_blocked);
+
+ debugfs_create_u32_array("histo_blocked", 0444, d_spin_debug,
+spinlock_stats.histo_spin_blocked, HISTO_BUCKETS + 1);
+
Please drop all of these and replace with tracepoints in the appropriate
spots. Everything else (including the historgram) can be reconstructed
the tracepoints in userspace.
I think Jeremy pointed that tracepoints use spinlocks and hence debugfs
is the option.. no ?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/14/2012 08:26 PM, Raghavendra K T wrote:
> Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
>
> During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has
> required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so,
> support for pv-ticketlocks is registered via pv_lock_ops.
>
> Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu.
> +
> + debugfs_create_u8("zero_stats", 0644, d_spin_debug, &zero_stats);
> +
> + debugfs_create_u32("taken_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW]);
> + debugfs_create_u32("taken_slow_pickup", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW_PICKUP]);
> +
> + debugfs_create_u32("released_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW]);
> + debugfs_create_u32("released_slow_kicked", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW_KICKED]);
> +
> + debugfs_create_u64("time_blocked", 0444, d_spin_debug,
> +&spinlock_stats.time_blocked);
> +
> + debugfs_create_u32_array("histo_blocked", 0444, d_spin_debug,
> + spinlock_stats.histo_spin_blocked, HISTO_BUCKETS + 1);
> +
>
Please drop all of these and replace with tracepoints in the appropriate
spots. Everything else (including the historgram) can be reconstructed
the tracepoints in userspace.
--
error compiling committee.c: too many arguments to function
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 01/16/2012 08:42 AM, Alexander Graf wrote: On 14.01.2012, at 19:26, Raghavendra K T wrote: Extends Linux guest running on KVM hypervisor to support pv-ticketlocks. During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so, support for pv-ticketlocks is registered via pv_lock_ops. Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu. Signed-off-by: Srivatsa Vaddagiri Signed-off-by: Suzuki Poulose Signed-off-by: Raghavendra K T --- diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h index 7a94987..cf5327c 100644 --- a/arch/x86/include/asm/kvm_para.h +++ b/arch/x86/include/asm/kvm_para.h @@ -195,10 +195,20 @@ void kvm_async_pf_task_wait(u32 token); void kvm_async_pf_task_wake(u32 token); [...] +} +#endif /* CONFIG_PARAVIRT_SPINLOCKS */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c7b05fc..4d7a950 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c This patch is mixing host and guest code. Please split those up. Agree. The host code should have gone to patch 2. Alex @@ -5754,8 +5754,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
On 14.01.2012, at 19:26, Raghavendra K T wrote:
> Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
>
> During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has
> required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so,
> support for pv-ticketlocks is registered via pv_lock_ops.
>
> Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu.
>
> Signed-off-by: Srivatsa Vaddagiri
> Signed-off-by: Suzuki Poulose
> Signed-off-by: Raghavendra K T
> ---
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 7a94987..cf5327c 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -195,10 +195,20 @@ void kvm_async_pf_task_wait(u32 token);
> void kvm_async_pf_task_wake(u32 token);
> u32 kvm_read_and_reset_pf_reason(void);
> extern void kvm_disable_steal_time(void);
> -#else
> -#define kvm_guest_init() do { } while (0)
> +
> +#ifdef CONFIG_PARAVIRT_SPINLOCKS
> +void __init kvm_spinlock_init(void);
> +#else /* CONFIG_PARAVIRT_SPINLOCKS */
> +static void kvm_spinlock_init(void)
> +{
> +}
> +#endif /* CONFIG_PARAVIRT_SPINLOCKS */
> +
> +#else /* CONFIG_KVM_GUEST */
> +#define kvm_guest_init() do {} while (0)
> #define kvm_async_pf_task_wait(T) do {} while(0)
> #define kvm_async_pf_task_wake(T) do {} while(0)
> +
> static inline u32 kvm_read_and_reset_pf_reason(void)
> {
> return 0;
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index a9c2116..ec55a0b 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -33,6 +33,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -545,6 +546,7 @@ static void __init kvm_smp_prepare_boot_cpu(void)
> #endif
> kvm_guest_cpu_init();
> native_smp_prepare_boot_cpu();
> + kvm_spinlock_init();
> }
>
> static void __cpuinit kvm_guest_cpu_online(void *dummy)
> @@ -627,3 +629,250 @@ static __init int activate_jump_labels(void)
> return 0;
> }
> arch_initcall(activate_jump_labels);
> +
> +#ifdef CONFIG_PARAVIRT_SPINLOCKS
> +
> +enum kvm_contention_stat {
> + TAKEN_SLOW,
> + TAKEN_SLOW_PICKUP,
> + RELEASED_SLOW,
> + RELEASED_SLOW_KICKED,
> + NR_CONTENTION_STATS
> +};
> +
> +#ifdef CONFIG_KVM_DEBUG_FS
> +
> +static struct kvm_spinlock_stats
> +{
> + u32 contention_stats[NR_CONTENTION_STATS];
> +
> +#define HISTO_BUCKETS30
> + u32 histo_spin_blocked[HISTO_BUCKETS+1];
> +
> + u64 time_blocked;
> +} spinlock_stats;
> +
> +static u8 zero_stats;
> +
> +static inline void check_zero(void)
> +{
> + u8 ret;
> + u8 old = ACCESS_ONCE(zero_stats);
> + if (unlikely(old)) {
> + ret = cmpxchg(&zero_stats, old, 0);
> + /* This ensures only one fellow resets the stat */
> + if (ret == old)
> + memset(&spinlock_stats, 0, sizeof(spinlock_stats));
> + }
> +}
> +
> +static inline void add_stats(enum kvm_contention_stat var, u32 val)
> +{
> + check_zero();
> + spinlock_stats.contention_stats[var] += val;
> +}
> +
> +
> +static inline u64 spin_time_start(void)
> +{
> + return sched_clock();
> +}
> +
> +static void __spin_time_accum(u64 delta, u32 *array)
> +{
> + unsigned index = ilog2(delta);
> +
> + check_zero();
> +
> + if (index < HISTO_BUCKETS)
> + array[index]++;
> + else
> + array[HISTO_BUCKETS]++;
> +}
> +
> +static inline void spin_time_accum_blocked(u64 start)
> +{
> + u32 delta = sched_clock() - start;
> +
> + __spin_time_accum(delta, spinlock_stats.histo_spin_blocked);
> + spinlock_stats.time_blocked += delta;
> +}
> +
> +static struct dentry *d_spin_debug;
> +static struct dentry *d_kvm_debug;
> +
> +struct dentry *kvm_init_debugfs(void)
> +{
> + d_kvm_debug = debugfs_create_dir("kvm", NULL);
> + if (!d_kvm_debug)
> + printk(KERN_WARNING "Could not create 'kvm' debugfs
> directory\n");
> +
> + return d_kvm_debug;
> +}
> +
> +static int __init kvm_spinlock_debugfs(void)
> +{
> + struct dentry *d_kvm = kvm_init_debugfs();
> +
> + if (d_kvm == NULL)
> + return -ENOMEM;
> +
> + d_spin_debug = debugfs_create_dir("spinlocks", d_kvm);
> +
> + debugfs_create_u8("zero_stats", 0644, d_spin_debug, &zero_stats);
> +
> + debugfs_create_u32("taken_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW]);
> + debugfs_create_u32("taken_slow_pickup", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[TAKEN_SLOW_PICKUP]);
> +
> + debugfs_create_u32("released_slow", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW]);
> + debugfs_create_u32("released_slow_kicked", 0444, d_spin_debug,
> +&spinlock_stats.contention_stats[RELEASED_SLOW_KICKED]);
> +
> + debugfs_create_u64("time_blocked", 0444, d_spin_debug,
> +
[PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor
Extends Linux guest running on KVM hypervisor to support pv-ticketlocks.
During smp_boot_cpus paravirtualied KVM guest detects if the hypervisor has
required feature (KVM_FEATURE_PVLOCK_KICK) to support pv-ticketlocks. If so,
support for pv-ticketlocks is registered via pv_lock_ops.
Use KVM_HC_KICK_CPU hypercall to wakeup waiting/halted vcpu.
Signed-off-by: Srivatsa Vaddagiri
Signed-off-by: Suzuki Poulose
Signed-off-by: Raghavendra K T
---
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 7a94987..cf5327c 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -195,10 +195,20 @@ void kvm_async_pf_task_wait(u32 token);
void kvm_async_pf_task_wake(u32 token);
u32 kvm_read_and_reset_pf_reason(void);
extern void kvm_disable_steal_time(void);
-#else
-#define kvm_guest_init() do { } while (0)
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+void __init kvm_spinlock_init(void);
+#else /* CONFIG_PARAVIRT_SPINLOCKS */
+static void kvm_spinlock_init(void)
+{
+}
+#endif /* CONFIG_PARAVIRT_SPINLOCKS */
+
+#else /* CONFIG_KVM_GUEST */
+#define kvm_guest_init() do {} while (0)
#define kvm_async_pf_task_wait(T) do {} while(0)
#define kvm_async_pf_task_wake(T) do {} while(0)
+
static inline u32 kvm_read_and_reset_pf_reason(void)
{
return 0;
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index a9c2116..ec55a0b 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -545,6 +546,7 @@ static void __init kvm_smp_prepare_boot_cpu(void)
#endif
kvm_guest_cpu_init();
native_smp_prepare_boot_cpu();
+ kvm_spinlock_init();
}
static void __cpuinit kvm_guest_cpu_online(void *dummy)
@@ -627,3 +629,250 @@ static __init int activate_jump_labels(void)
return 0;
}
arch_initcall(activate_jump_labels);
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+
+enum kvm_contention_stat {
+ TAKEN_SLOW,
+ TAKEN_SLOW_PICKUP,
+ RELEASED_SLOW,
+ RELEASED_SLOW_KICKED,
+ NR_CONTENTION_STATS
+};
+
+#ifdef CONFIG_KVM_DEBUG_FS
+
+static struct kvm_spinlock_stats
+{
+ u32 contention_stats[NR_CONTENTION_STATS];
+
+#define HISTO_BUCKETS 30
+ u32 histo_spin_blocked[HISTO_BUCKETS+1];
+
+ u64 time_blocked;
+} spinlock_stats;
+
+static u8 zero_stats;
+
+static inline void check_zero(void)
+{
+ u8 ret;
+ u8 old = ACCESS_ONCE(zero_stats);
+ if (unlikely(old)) {
+ ret = cmpxchg(&zero_stats, old, 0);
+ /* This ensures only one fellow resets the stat */
+ if (ret == old)
+ memset(&spinlock_stats, 0, sizeof(spinlock_stats));
+ }
+}
+
+static inline void add_stats(enum kvm_contention_stat var, u32 val)
+{
+ check_zero();
+ spinlock_stats.contention_stats[var] += val;
+}
+
+
+static inline u64 spin_time_start(void)
+{
+ return sched_clock();
+}
+
+static void __spin_time_accum(u64 delta, u32 *array)
+{
+ unsigned index = ilog2(delta);
+
+ check_zero();
+
+ if (index < HISTO_BUCKETS)
+ array[index]++;
+ else
+ array[HISTO_BUCKETS]++;
+}
+
+static inline void spin_time_accum_blocked(u64 start)
+{
+ u32 delta = sched_clock() - start;
+
+ __spin_time_accum(delta, spinlock_stats.histo_spin_blocked);
+ spinlock_stats.time_blocked += delta;
+}
+
+static struct dentry *d_spin_debug;
+static struct dentry *d_kvm_debug;
+
+struct dentry *kvm_init_debugfs(void)
+{
+ d_kvm_debug = debugfs_create_dir("kvm", NULL);
+ if (!d_kvm_debug)
+ printk(KERN_WARNING "Could not create 'kvm' debugfs
directory\n");
+
+ return d_kvm_debug;
+}
+
+static int __init kvm_spinlock_debugfs(void)
+{
+ struct dentry *d_kvm = kvm_init_debugfs();
+
+ if (d_kvm == NULL)
+ return -ENOMEM;
+
+ d_spin_debug = debugfs_create_dir("spinlocks", d_kvm);
+
+ debugfs_create_u8("zero_stats", 0644, d_spin_debug, &zero_stats);
+
+ debugfs_create_u32("taken_slow", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[TAKEN_SLOW]);
+ debugfs_create_u32("taken_slow_pickup", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[TAKEN_SLOW_PICKUP]);
+
+ debugfs_create_u32("released_slow", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[RELEASED_SLOW]);
+ debugfs_create_u32("released_slow_kicked", 0444, d_spin_debug,
+ &spinlock_stats.contention_stats[RELEASED_SLOW_KICKED]);
+
+ debugfs_create_u64("time_blocked", 0444, d_spin_debug,
+ &spinlock_stats.time_blocked);
+
+ debugfs_create_u32_array("histo_blocked", 0444, d_spin_debug,
+spinlock_stats.histo_spin_blocked, HISTO_BUCKETS + 1);
+
+ return 0;
+}
+fs_initcall(kvm_spinlock_debugfs);
+#else /* !CONFIG_KVM_DEBUG_F
