Patchwork [1/3] powerpc/kvm: simplify the entering logic for secondary thread

login
register
mail settings
Submitter Liu Ping Fan
Date Nov. 5, 2013, 7:42 a.m.
Message ID <1383637364-14691-1-git-send-email-pingfank@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/288399/
State New
Headers show

Comments

Liu Ping Fan - Nov. 5, 2013, 7:42 a.m.
After the primary vcpu changes vcore_state to VCORE_RUNNING, there is
very little chance to schedule to secondary vcpu. So if we change the
code sequence around set vcore_state to VCORE_RUNNING and disable
preemption, we lost little. But we simplify the entering logi, based on
the fact that if primary vcpu runs, the secondary vcpu can not be scheduled.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 arch/powerpc/kvm/book3s_hv.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
Paul Mackerras - Nov. 6, 2013, 5:01 a.m.
On Tue, Nov 05, 2013 at 03:42:42PM +0800, Liu Ping Fan wrote:
> After the primary vcpu changes vcore_state to VCORE_RUNNING, there is
> very little chance to schedule to secondary vcpu. So if we change the

Why do you say there is very little chance to run the secondary vcpu?

> code sequence around set vcore_state to VCORE_RUNNING and disable
> preemption, we lost little. But we simplify the entering logi, based on
> the fact that if primary vcpu runs, the secondary vcpu can not be scheduled.

If a vcpu does a hypercall or something else that requires the host
(kernel or userspace) to do something, that can happen in the context
of the vcpu task for that vcpu.  That vcpu task can run on another
core (unless it has been pinned).  When it is finished we would like
the vcpu to continue executing in the guest as soon as possible.  The
code that you remove in this patch enables that to happen without
having to wait until the other threads exit the guest.  So I don't
think it is a good idea to remove this code.

Regards,
Paul.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Ping Fan - Nov. 6, 2013, 7:26 a.m.
On Wed, Nov 6, 2013 at 1:01 PM, Paul Mackerras <paulus@samba.org> wrote:
> On Tue, Nov 05, 2013 at 03:42:42PM +0800, Liu Ping Fan wrote:
>> After the primary vcpu changes vcore_state to VCORE_RUNNING, there is
>> very little chance to schedule to secondary vcpu. So if we change the
>
> Why do you say there is very little chance to run the secondary vcpu?
>
>> code sequence around set vcore_state to VCORE_RUNNING and disable
>> preemption, we lost little. But we simplify the entering logi, based on
>> the fact that if primary vcpu runs, the secondary vcpu can not be scheduled.
>
> If a vcpu does a hypercall or something else that requires the host
> (kernel or userspace) to do something, that can happen in the context
> of the vcpu task for that vcpu.  That vcpu task can run on another
> core (unless it has been pinned).  When it is finished we would like
> the vcpu to continue executing in the guest as soon as possible.  The
> code that you remove in this patch enables that to happen without
> having to wait until the other threads exit the guest.  So I don't
> think it is a good idea to remove this code.
>
Yes, you are right.

Thanks and regards,
Pingfan


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

Patch

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 62a2b5a..38b1fc0 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1222,8 +1222,8 @@  static void kvmppc_run_core(struct kvmppc_vcore *vc)
 		kvmppc_create_dtl_entry(vcpu, vc);
 	}
 
-	vc->vcore_state = VCORE_RUNNING;
 	preempt_disable();
+	vc->vcore_state = VCORE_RUNNING;
 	spin_unlock(&vc->lock);
 
 	kvm_guest_enter();
@@ -1351,12 +1351,7 @@  static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 	 * this thread straight away and have it join in.
 	 */
 	if (!signal_pending(current)) {
-		if (vc->vcore_state == VCORE_RUNNING &&
-		    VCORE_EXIT_COUNT(vc) == 0) {
-			vcpu->arch.ptid = vc->n_runnable - 1;
-			kvmppc_create_dtl_entry(vcpu, vc);
-			kvmppc_start_thread(vcpu);
-		} else if (vc->vcore_state == VCORE_SLEEPING) {
+		if (vc->vcore_state == VCORE_SLEEPING) {
 			wake_up(&vc->wq);
 		}