Messages in this thread | | | Date | Fri, 28 Sep 2012 11:46:39 +0530 | From | Raghavendra K T <> | Subject | Re: [PATCH RFC 1/2] kvm: Handle undercommitted guest case in PLE handler |
| |
On 09/28/2012 02:37 AM, Jiannan Ouyang wrote: > > > On Thu, Sep 27, 2012 at 4:50 AM, Avi Kivity <avi@redhat.com > <mailto:avi@redhat.com>> wrote: > > On 09/25/2012 04:43 PM, Jiannan Ouyang wrote: > > I've actually implemented this preempted_bitmap idea. > > Interesting, please share the code if you can. > > > However, I'm doing this to expose this information to the guest, > so the > > guest is able to know if the lock holder is preempted or not before > > spining. Right now, I'm doing experiment to show that this idea > works. > > > > I'm wondering what do you guys think of the relationship between the > > pv_ticketlock approach and PLE handler approach. Are we going to > adopt > > PLE instead of the pv ticketlock, and why? > > Right now we're searching for the best solution. The tradeoffs are more > or less: > > PLE: > - works for unmodified / non-Linux guests > - works for all types of spins (e.g. smp_call_function*()) > - utilizes an existing hardware interface (PAUSE instruction) so likely > more robust compared to a software interface > > PV: > - has more information, so it can perform better > > Given these tradeoffs, if we can get PLE to work for moderate amounts of > overcommit then I'll prefer it (even if it slightly underperforms PV). > If we are unable to make it work well, then we'll have to add PV. > > -- > error compiling committee.c: too many arguments to function > > > FYI. The preempted_bitmap patch. > > I delete some unrelated code in the generated patch file and seems > broken the patch file format... I hope anyone could teach me some > solutions. > However, it's pretty straight forward, four things: declaration, > initialization, set and clear. I think you guys can figure it out easily! > > As Avi sugguested, you could check task state TASK_RUNNING in sched_out. > > Signed-off-by: Jiannan Ouyang <ouyang@cs.pitt.edu > <mailto:ouyang@cs.pitt.edu>> > > diff --git a/arch/x86/include/asm/ > > paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > index 8613cbb..4fcb648 100644 > --- a/arch/x86/include/asm/paravirt_types.h > +++ b/arch/x86/include/asm/paravirt_types.h > @@ -73,6 +73,16 @@ struct pv_info { > const char *name; > };
I suppose we need this in common place since s390 also should have this, if we are using this information in vcpu_on_spin()..
> > +struct pv_sched_info { > + unsigned long sched_bitmap;
Thinking, whether we need something similar to cpumask here? Only thing is we are representing guest (v)cpumask.
> +} __attribute__((__packed__)); > + > struct pv_init_ops { > /* > * Patch may replace one of the defined code sequences with > diff --git a/arch/x86/kernel/paravirt-spinlocks.c > b/arch/x86/kernel/paravirt-spinlocks.c > index 676b8c7..2242d22 100644 > --- a/arch/x86/kernel/paravirt-spinlocks.c > +++ b/arch/x86/kernel/paravirt-spinlocks.c > > +struct pv_sched_info pv_sched_info = { > + .sched_bitmap = (unsigned long)-1, > +}; > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 44ee712..3eb277e 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -494,6 +494,11 @@ static struct kvm *kvm_create_vm(unsigned long > type) > mutex_init(&kvm->slots_lock); > atomic_set(&kvm->users_count, 1); > > +#ifdef CONFIG_PARAVIRT_SPINLOCKS > + kvm->pv_sched_info.sched_bitmap = (unsigned long)-1; > +#endif > + > r = kvm_init_mmu_notifier(kvm); > if (r) > goto out_err; > @@ -2697,7 +2702,13 @@ struct kvm_vcpu > *preempt_notifier_to_vcpu(struct preempt_notifier *pn) > static void kvm_sched_in(struct preempt_notifier *pn, int cpu) > { > struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn); > > + set_bit(vcpu->vcpu_id, &vcpu->kvm->pv_sched_info.sched_bitmap); > kvm_arch_vcpu_load(vcpu, cpu); > } > > @@ -2705,7 +2716,13 @@ static void kvm_sched_out(struct > preempt_notifier *pn, > struct task_struct *next) > { > struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn); > > + clear_bit(vcpu->vcpu_id, > &vcpu->kvm->pv_sched_info.sched_bitmap); > kvm_arch_vcpu_put(vcpu); > }
| |