Re: [PATCH 1/3] kvm/ppc/booke: Hold srcu lock when calling gfn functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/01/2013 07:15:53 PM, Marcelo Tosatti wrote:
On Fri, Apr 26, 2013 at 07:53:38PM -0500, Scott Wood wrote:
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 1020119..506c87d 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -832,6 +832,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  {
>  	int r = RESUME_HOST;
>  	int s;
> +	int idx = 0; /* silence bogus uninitialized warning */
> +	bool need_srcu = false;
>
>  	/* update before a new last_exit_type is rewritten */
>  	kvmppc_update_timing_stats(vcpu);
> @@ -847,6 +849,20 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
>  	run->exit_reason = KVM_EXIT_UNKNOWN;
>  	run->ready_for_interrupt_injection = 1;
>
> +	/*
> +	 * Don't get the srcu lock unconditionally, because kvm_ppc_pv()
> +	 * can call kvm_vcpu_block(), and kvm_ppc_pv() is shared with
> +	 * book3s, so dropping the srcu lock there would be awkward.
> +	 */
> +	switch (exit_nr) {
> +	case BOOKE_INTERRUPT_ITLB_MISS:
> +	case BOOKE_INTERRUPT_DTLB_MISS:
> +		need_srcu = true;
> +	}

This is not good practice (codepaths should either hold srcu or not hold
it, unconditionally).

How is it different from moving the srcu lock into individual cases of the switch? I just did it this way to make it easier to add new exception types if necessary (e.g. at the time I thought I'd end up adding exceptions which lead to instruction emulation, but I ended up acquiring the lock further down the path in that case).

Can you give more details of the issue? (not obvious)

ITLB/DTLB miss call things like gfn_to_memslot() which need the lock (but don't grab it themselves -- that seems like the real bad practice here...). The syscall exceptions can't have the SRCU lock held, because they call kvmppc_kvm_pv which can call kvm_vcpu_block() (yes, you can sleep with SRCU, but not indefinitely...). kvmppc_kvm_pv is shared with book3s code, so adding code to drop the srcu lock there would be a problem since book3s doesn't hold the SRCU lock then...

-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux