kvm: Improving directed yield in PLE handler

Signed-off-by Raghavendra K T<[email protected]>

Noting pause loop exited vcpu helps in filtering right candidate to yield. Yielding to same vcpu may result in more wastage of cpu.

From: Raghavendra K T<[email protected]>
---

arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 492

Quoted 10 lines · Raghavendra K T

arch/x86/include/asm/kvm_host.h +6 -0

@@ -484,6 +484,13 @@ struct kvm_vcpu_arch {
 u64 length;
 u64 status;
 } osvw;
+
+ /* Pause loop exit optimization */
+ struct {
+ bool pause_loop_exited;
+ bool dy_eligible;
+ } plo;

I know kvm_vcpu_arch is traditionally not a well documented structure, but it would be really nice if each variable inside this sub-structure could get some documentation.

Also, do we really want to introduce another acronym here?

Or would we be better off simply calling this struct .ple, since that is a name people are already familiar with.

Signed-off-by Raghavendra K T <[email protected]>

Noting pause loop exited vcpu helps in filtering right candidate to yield. Yielding to same vcpu may result in more wastage of cpu.

struct kvm_lpage_info {

arch/x86/kvm/svm.c +1 -0

index f75af40..a492f5d 100644
@@ -3264,6 +3264,7 @@ static int interrupt_window_interception(struct vcpu_svm *svm)
 
 static int pause_interception(struct vcpu_svm *svm)
 {
+	svm->vcpu.arch.plo.pause_loop_exited = true;
 	kvm_vcpu_on_spin(&(svm->vcpu));
 	return 1;
 }

arch/x86/kvm/vmx.c +0 -0

index 32eb588..600fb3c 100644

arch/x86/kvm/x86.c +0 -0

 	kvm_guest_enter();
 

Quoted 24 lines · Raghavendra K T

Signed-off-by Raghavendra K T<[email protected]>

Noting pause loop exited vcpu helps in filtering right candidate to yield. Yielding to same vcpu may result in more wastage of cpu.

  struct kvm_lpage_info {

arch/x86/kvm/svm.c +1 -0

index f75af40..a492f5d 100644
@@ -3264,6 +3264,7 @@ static int interrupt_window_interception(struct vcpu_svm *svm)
 
  static int pause_interception(struct vcpu_svm *svm)
  {
+	svm->vcpu.arch.plo.pause_loop_exited = true;
  	kvm_vcpu_on_spin(&(svm->vcpu));
  	return 1;
  }

arch/x86/kvm/vmx.c +0 -0

index 32eb588..600fb3c 100644

arch/x86/kvm/vmx.c out: · line 4950

Quoted 6 lines

arch/x86/kvm/vmx.c +1 -0

@@ -4945,6 +4945,7 @@ out:
  static int handle_pause(struct kvm_vcpu *vcpu)
  {
  	skip_emulated_instruction(vcpu);
+	vcpu->arch.plo.pause_loop_exited = true;
  	kvm_vcpu_on_spin(vcpu);
 

This code is duplicated. Should we move it to kvm_vcpu_on_spin?

That means the .plo structure needs to be in common code, but that's not too bad perhaps.

Quoted 5 lines

index be6d549..07dbd14 100644

arch/x86/kvm/x86.c +0 -0

@@ -5331,7 +5331,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_vcpu *vcpu) · line 5334

Quoted 4 lines

arch/x86/kvm/x86.c +1 -1

  	if (req_immediate_exit)
  		smp_send_reschedule(vcpu->cpu);
-
+	vcpu->arch.plo.pause_loop_exited = false;

This adds some tiny overhead to vcpu entry. You could remove it by using the vcpu->requests mechanism to clear the flag, since vcpu->requests is already checked on every entry.

Quoted 3 lines

arch/x86/kvm/x86.c +0 -0

  	kvm_guest_enter();
 

Quoted 36 lines · Avi Kivity

Quoted 26 lines · Raghavendra K T

Signed-off-by Raghavendra K T<[email protected]>

Noting pause loop exited vcpu helps in filtering right candidate to yield.
Yielding to same vcpu may result in more wastage of cpu.

  struct kvm_lpage_info {

arch/x86/kvm/svm.c +1 -0

index f75af40..a492f5d 100644
@@ -3264,6 +3264,7 @@ static int interrupt_window_interception(struct
 vcpu_svm *svm)
 
  static int pause_interception(struct vcpu_svm *svm)
  {
+    svm->vcpu.arch.plo.pause_loop_exited = true;
      kvm_vcpu_on_spin(&(svm->vcpu));
      return 1;

  }

arch/x86/kvm/vmx.c +0 -0

index 32eb588..600fb3c 100644

arch/x86/kvm/vmx.c out: · line 4950

Quoted 6 lines

arch/x86/kvm/vmx.c +1 -0

@@ -4945,6 +4945,7 @@ out:
  static int handle_pause(struct kvm_vcpu *vcpu)
  {
      skip_emulated_instruction(vcpu);
+    vcpu->arch.plo.pause_loop_exited = true;
      kvm_vcpu_on_spin(vcpu);
 

This code is duplicated. Should we move it to kvm_vcpu_on_spin?

Quoted 3 lines

That means the .plo structure needs to be in common code, but that's not too bad perhaps.

Since PLE is very much tied to x86, and proposed changes are very much specific to PLE handler, I thought it is better to make arch specific.

So do you think it is good to move inside vcpu_on_spin and make ple structure belong to common code?

Quoted 4 lines

This adds some tiny overhead to vcpu entry. You could remove it by using the vcpu->requests mechanism to clear the flag, since vcpu->requests is already checked on every entry.

So IIUC, let's have request bit for indicating PLE,

pause_interception() /handle_pause()
{
make_request(PLE_REQUEST)
vcpu_on_spin()

}

check_eligibility()
{
!test_request(PLE_REQUEST) || ( test_request(PLE_REQUEST) &&
dy_eligible())
.
.
}

vcpu_run()
{

check_request(PLE_REQUEST)
.
.
}

Is this is the expected flow you had in mind?

[ But my only concern was not resetting for cases where we do not do guest_enter(). will test how that goes].

Quoted 41 lines · Raghavendra K T

Quoted 39 lines · Avi Kivity

Quoted 26 lines · Raghavendra K T

Signed-off-by Raghavendra K T<[email protected]>

Noting pause loop exited vcpu helps in filtering right candidate to yield.
Yielding to same vcpu may result in more wastage of cpu.

   struct kvm_lpage_info {

arch/x86/kvm/svm.c +1 -0

index f75af40..a492f5d 100644
@@ -3264,6 +3264,7 @@ static int interrupt_window_interception(struct
 vcpu_svm *svm)
 
   static int pause_interception(struct vcpu_svm *svm)
   {
+    svm->vcpu.arch.plo.pause_loop_exited = true;
       kvm_vcpu_on_spin(&(svm->vcpu));
       return 1;

   }

arch/x86/kvm/vmx.c +0 -0

index 32eb588..600fb3c 100644

arch/x86/kvm/vmx.c out: · line 4950

Quoted 6 lines

arch/x86/kvm/vmx.c +1 -0

@@ -4945,6 +4945,7 @@ out:
   static int handle_pause(struct kvm_vcpu *vcpu)
   {
       skip_emulated_instruction(vcpu);
+    vcpu->arch.plo.pause_loop_exited = true;
       kvm_vcpu_on_spin(vcpu);
 

This code is duplicated. Should we move it to kvm_vcpu_on_spin?

That means the .plo structure needs to be in common code, but that's not too bad perhaps.

Quoted 5 lines

Since PLE is very much tied to x86, and proposed changes are very much specific to PLE handler, I thought it is better to make arch specific.

So do you think it is good to move inside vcpu_on_spin and make ple structure belong to common code?

See the discussion with Christian. PLE is tied to x86, but cpu_relax() and facilities to trap it are not.

Quoted 26 lines

Quoted 5 lines

This adds some tiny overhead to vcpu entry. You could remove it by using the vcpu->requests mechanism to clear the flag, since vcpu->requests is already checked on every entry.

So IIUC, let's have request bit for indicating PLE,

pause_interception() /handle_pause()
{
make_request(PLE_REQUEST)
vcpu_on_spin()

}

check_eligibility()
{
!test_request(PLE_REQUEST) || ( test_request(PLE_REQUEST)&&
dy_eligible())
.
.
}

vcpu_run()
{

Quoted 6 lines

check_request(PLE_REQUEST)
.
.
}

Is this is the expected flow you had in mind?

Yes, something like that.

Quoted 4 lines

[ But my only concern was not resetting for cases where we do not do guest_enter(). will test how that goes].

Hm, suppose we're the next-in-line for a ticket lock and exit due to PLE. The lock holder completes and unlocks, which really assigns the lock to us. So now we are the lock owner, yet we are marked as don't yield-to-us in the PLE code.

Quoted 8 lines

Hm, suppose we're the next-in-line for a ticket lock and exit due to
PLE. The lock holder completes and unlocks, which really assigns the
lock to us. So now we are the lock owner, yet we are marked as don't
yield-to-us in the PLE code.

Yes.. off-topic but that is solved by kicked flag in PV spinlocks.

Quoted 4 lines · Avi Kivity

This adds some tiny overhead to vcpu entry. You could remove it by using the vcpu->requests mechanism to clear the flag, since vcpu->requests is already checked on every entry.

So IIUC, let's have request bit for indicating PLE,

pause_interception() /handle_pause()
{
make_request(PLE_REQUEST)
vcpu_on_spin()

}

check_eligibility()
{
!test_request(PLE_REQUEST) || ( test_request(PLE_REQUEST) &&
dy_eligible())
.
.
}

vcpu_run()
{

check_request(PLE_REQUEST)

Quoted 23 lines · Raghavendra K T

Quoted 4 lines · Avi Kivity

This adds some tiny overhead to vcpu entry. You could remove it by using the vcpu->requests mechanism to clear the flag, since vcpu->requests is already checked on every entry.

So IIUC, let's have request bit for indicating PLE,

pause_interception() /handle_pause()

{
   make_request(PLE_REQUEST)
   vcpu_on_spin()

}

check_eligibility()

   {
   !test_request(PLE_REQUEST) || ( test_request(PLE_REQUEST)&&

dy_eligible())
.
.
}

Quoted 5 lines

vcpu_run()
{

check_request(PLE_REQUEST)

I know check_request will clear PLE_REQUEST, but you just need a clear_request here, right?

arch/x86/kvm/x86.c bool kvm_arch_can_inject_async_page_present(struct kvm_vcpu *vcpu) · line 6638

Quoted 13 lines · Raghavendra K T

arch/x86/kvm/x86.c +13 -0

+bool kvm_arch_vcpu_check_and_update_eligible(struct kvm_vcpu *vcpu)
+{
+ bool eligible;
+
+ eligible = !vcpu->arch.plo.pause_loop_exited ||
+ (vcpu->arch.plo.pause_loop_exited&&
+ vcpu->arch.plo.dy_eligible);
+
+ if (vcpu->arch.plo.pause_loop_exited)
+ vcpu->arch.plo.dy_eligible = !vcpu->arch.plo.dy_eligible;
+
+ return eligible;
+}

This is a nice simple mechanism to skip CPUs that were eligible last time and had pause loop exits recently.

However, it could stand some documentation. Please add a good comment explaining how and why the algorithm works, when arch.plo.pause_loop_exited is cleared, etc...

It would be good to make this heuristic understandable to people who look at the code for the first time.

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

Quoted 10 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

We could do the same for s390. The appropriate exit would be diag44 (yield to hypervisor).

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though. So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 8 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Quoted 2 lines

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

We could do the same for s390. The appropriate exit would be diag44 (yield to hypervisor).

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 6 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 4 lines

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Ignoring the name,

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

So this would probably improve guests that uses cpu_relax, for example stop_machine_run. I have no measurements, though.

Quoted 6 lines · Christian Borntraeger

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

Quoted 7 lines

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

We could make it available via shared memory, updated using preempt notifiers. Of course piling on more pv makes this less attractive.

Quoted 9 lines

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 6 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 6 lines

Quoted 4 lines

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Ignoring the name,

What name would you suggest?

Quoted 4 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit,

and the way it implements cpu_relax() by adjusting hw thread priority,

Quoted 8 lines

Quoted 5 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit,

What dislike of overcommit?

Quoted 2 lines

and the way it implements cpu_relax() by adjusting hw thread priority,

Yeah, I don't think we can intercept relaxing.

It's basically a nop-like instruction that gives hardware hints on its current priorities.

That said, we can always add PV code.

Quoted 6 lines · Alexander Graf

Quoted 6 lines

Quoted 5 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

Quoted 4 lines

Quoted 2 lines

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit,

What dislike of overcommit?

I understood ppc virtualization is more of the partitioning sort. Perhaps I misunderstood it. But the reliance on device assignment, the restrictions on scheduling, etc. all point to it.

Quoted 5 lines

Quoted 3 lines

and the way it implements cpu_relax() by adjusting hw thread priority,

Yeah, I don't think we can intercept relaxing.

... and the lack of ability to intercept cpu_relax() ...

Quoted 1 line

It's basically a nop-like instruction that gives hardware hints on its current priorities.

That's what x88 PAUSE does. But we can intercept it (and not just any execution - we can restrict intercept to tight loops executed more than a specific number of times).

Quoted 2 lines

That said, we can always add PV code.

Sure, but that's defeated by advancements like self-tuning PLE exits. It's hard to get this right.

Quoted 12 lines · Avi Kivity

Quoted 10 lines · Alexander Graf

Quoted 8 lines

Quoted 5 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit,

What dislike of overcommit?

Quoted 3 lines

I understood ppc virtualization is more of the partitioning sort. Perhaps I misunderstood it. But the reliance on device assignment, the restrictions on scheduling, etc. all point to it.

Well, you need to distinguish the different PPC targets here. In the embedded world, partitioning is obviously the biggest use case, though overcommit is possible. For big servers however, we usually do want overcommit and we do support it within the constraints hardware gives us.

It's really no different from x86 when it comes to the use case wideness :).

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

and the way it implements cpu_relax() by adjusting hw thread priority,

Yeah, I don't think we can intercept relaxing.

... and the lack of ability to intercept cpu_relax() ...

Quoted 5 lines

Quoted 1 line

It's basically a nop-like instruction that gives hardware hints on its current priorities.

That's what x88 PAUSE does. But we can intercept it (and not just any execution - we can restrict intercept to tight loops executed more than a specific number of times).

Yeah, it's pretty hard to fetch that information from PPC, since unlike x86 we split the hint from the loop. But I'll let Ben speak to that, he certainly knows way better how the hardware works.

Quoted 6 lines

Quoted 3 lines

That said, we can always add PV code.

Sure, but that's defeated by advancements like self-tuning PLE exits. It's hard to get this right.

Well, eventually everything we do in PV is going to be moot as soon as hardware catches up. In most cases from what I've seen it's only useful as an interim solution. But for that time it's good to have :).

Quoted 6 lines · Alexander Graf

Quoted 6 lines

Quoted 5 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

Quoted 4 lines

Quoted 2 lines

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit,

What dislike of overcommit?

I understood ppc virtualization is more of the partitioning sort. Perhaps I misunderstood it. But the reliance on device assignment, the restrictions on scheduling, etc. all point to it.

Quoted 3 lines

Quoted 1 line

and the way it implements cpu_relax() by adjusting hw thread priority,

Yeah, I don't think we can intercept relaxing.

... and the lack of ability to intercept cpu_relax() ...

Quoted 1 line

It's basically a nop-like instruction that gives hardware hints on its current priorities.

That's what x88 PAUSE does. But we can intercept it (and not just any execution - we can restrict intercept to tight loops executed more than a specific number of times).

Quoted 1 line

That said, we can always add PV code.

Sure, but that's defeated by advancements like self-tuning PLE exits. It's hard to get this right.

Quoted 6 lines · Christian Borntraeger

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

Quoted 7 lines

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

We could make it available via shared memory, updated using preempt notifiers. Of course piling on more pv makes this less attractive.

Quoted 9 lines

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 6 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 6 lines

Quoted 4 lines

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Ignoring the name,

What name would you suggest?

Quoted 5 lines

yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work. If we move that to common code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit, and the way it implements cpu_relax() by adjusting hw thread priority, I'm guessing it doesn't intercept those either, but I'm copying the ppc people in case I'm wrong. So it's s390 and x86.

Quoted 2 lines

So this would probably improve guests that uses cpu_relax, for example stop_machine_run. I have no measurements, though.

smp_call_function() too (though that can be converted to directed yield too). It seems worthwhile.

Quoted 32 lines · Avi Kivity

Quoted 6 lines · Christian Borntraeger

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

Quoted 7 lines

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

We could make it available via shared memory, updated using preempt notifiers. Of course piling on more pv makes this less attractive.

Quoted 14 lines

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 10 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Quoted 3 lines

Quoted 1 line

Ignoring the name,

What name would you suggest?

maybe vcpu_no_progress instead of pause_loop_exited

Quoted 33 lines · Christian Borntraeger

Quoted 32 lines · Avi Kivity

Quoted 6 lines · Christian Borntraeger

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

Quoted 7 lines

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

We could make it available via shared memory, updated using preempt notifiers. Of course piling on more pv makes this less attractive.

Quoted 14 lines

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 10 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Ignoring the name,

What name would you suggest?

maybe vcpu_no_progress instead of pause_loop_exited

Ah, I thouht you objected to the CONFIG var. Maybe call it cpu_relax_intercepted since that's the linuxy name for the instruction.

Quoted 3 lines

Ah, I thouht you objected to the CONFIG var. Maybe call it cpu_relax_intercepted since that's the linuxy name for the instruction.

Ok, just to be on same page. 'll have :
1. cpu_relax_intercepted instead of pause_loop_exited.

2. CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT which is unconditionally selected for x86 and s390

3. make request mechanism to clear cpu_relax_intercepted.

('ll do same thing for s390 also but have not seen s390 code using request mechanism, so not sure if it ok.. otherwise we have to clear unconditionally for s390 before guest enter and for x86 we have to move make_request back to vmx/svm).
will post V3 with these changes.

Quoted 12 lines · Raghavendra K T

Quoted 3 lines

Ah, I thouht you objected to the CONFIG var. Maybe call it cpu_relax_intercepted since that's the linuxy name for the instruction.

Ok, just to be on same page. 'll have :
1. cpu_relax_intercepted instead of pause_loop_exited.

2. CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT which is unconditionally selected for x86 and s390

3. make request mechanism to clear cpu_relax_intercepted.

Quoted 5 lines

('ll do same thing for s390 also but have not seen s390 code using request mechanism, so not sure if it ok.. otherwise we have to clear unconditionally for s390 before guest enter and for x86 we have to move make_request back to vmx/svm).
will post V3 with these changes.

You can leave the s390 changes to the s390 people; just make sure the generic code is ready.

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit, and the way it implements cpu_relax() by adjusting hw thread priority, I'm guessing it doesn't intercept those either, but I'm copying the ppc people in case I'm wrong. So it's s390 and x86.

Quoted 5 lines

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit, and the way it implements cpu_relax() by adjusting hw thread priority, I'm guessing it doesn't intercept those either, but I'm copying the ppc people in case I'm wrong. So it's s390 and x86.

No but our spinlocks call __spin_yield() (or __rw_yield) which does some paravirt tricks already.

We check if the holder is currently running, and if not, we call the H_CONFER hypercall which can be used to "give" our time slice to the holder.

Our implementation of H_CONFER in KVM is currently a nop though.

Quoted 9 lines · Benjamin Herrenschmidt

Quoted 5 lines

ARM doesn't have an instruction for cpu_relax(), so it can't intercept it. Given ppc's dislike of overcommit, and the way it implements cpu_relax() by adjusting hw thread priority, I'm guessing it doesn't intercept those either, but I'm copying the ppc people in case I'm wrong. So it's s390 and x86.

No but our spinlocks call __spin_yield() (or __rw_yield) which does some paravirt tricks already.

Quoted 5 lines

We check if the holder is currently running, and if not, we call the H_CONFER hypercall which can be used to "give" our time slice to the holder.

Our implementation of H_CONFER in KVM is currently a nop though.

Okay, so you can join the party. See yield_to() and kvm_vcpu_on_spin().

Quoted 3 lines

So this would probably improve guests that uses cpu_relax, for example stop_machine_run. I have no measurements, though.

smp_call_function() too (though that can be converted to directed yield too). It seems worthwhile.

On 11/07/12 11:06, Avi Kivity wrote:
[...]

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

- if lock holder is not running and we looped for a while do a directed yield to that cpu.

Quoted 6 lines

Quoted 5 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax. I have to double check with others, if these cases are critical, but for now, it seems that your dummy implementation for s390 is just fine. After all it is a no-op until we implement something.

Quoted 4 lines

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Ignoring the name, yes the data structure itself seems based on the algorithm and not on arch specific things. That should work.

code then s390 will use that scheme automatically for the cases were we call kvm_vcpu_on_spin(). All others archs as well.

So this would probably improve guests that uses cpu_relax, for example stop_machine_run. I have no measurements, though.

Christian

Quoted 11 lines

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

Unfortunately we do not have information on lock-holder.

Quoted 12 lines · Raghavendra K T

Quoted 11 lines

Quoted 3 lines

Quoted 1 line

Almost all s390 kernels use diag9c (directed yield to a given guest cpu) for spinlocks, though.

Perhaps x86 should copy this.

See arch/s390/lib/spinlock.c
The basic idea is using several heuristics:
- loop for a given amount of loops
- check if the lock holder is currently scheduled by the hypervisor (smp_vcpu_scheduled, which uses the sigp sense running instruction) Dont know if such thing is available for x86. It must be a lot cheaper than a guest exit to be useful

Quoted 1 line

Unfortunately we do not have information on lock-holder.

That would be an independent patch and requires guest changes.

Quoted 1 line · Christian Borntraeger

On 11/07/12 11:06, Avi Kivity wrote:

Quoted 13 lines

Quoted 12 lines

Quoted 7 lines

So there is no win here, but there are other cases were diag44 is used, e.g. cpu_relax.
I have to double check with others, if these cases are critical, but for now, it seems
that your dummy implementation for s390 is just fine. After all it is a no-op until
we implement something.

Does the data structure make sense for you? If so we can move it to common code (and manage it in kvm_vcpu_on_spin()). We can guard it with CONFIG_KVM_HAVE_CPU_RELAX_INTERCEPT or something, so other archs don't have to pay anything.

Quoted 3 lines

Ignoring the name, yes the data structure itself seems based on the algorithm
and not on arch specific things. That should work.

Ok. can you please elaborate, on the flow.

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

(a) Not done PLE exit at all (probably he is preempted lock-holder)

(b) VCPU skipped in last iteration because it did PL exit, and probably has become eligible now (next eligible lock holder)

Future enhancemnets:
(1) Currently we have a boolean to decide on eligibility of vcpu. It would be nice if I get feedback on guest (>32 vcpu) whether we can improve better with integer counter. (with counter = say f(log n )).

(2) We have not considered system load during iteration of vcpu. With that information we can limit the scan and also decide whether schedule() is better. [ I am able to use #kicked vcpus to decide on this But may be there are better ideas like information from global loadavg.]

(3) We can exploit this further with PV patches since it also knows about next eligible lock-holder.

Summary: There is a huge improvement for moderate / no overcommit scenario
for kvm based guest on PLE machine (which is difficult ;) ).

Result:
Base : kernel 3.5.0-rc5 with Rik's Ple handler fix

Machine : Intel(R) Xeon(R) CPU X7560 @ 2.27GHz, 4 numa node, 256GB RAM,
32 core machine

Host: enterprise linux gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
with test kernels

Guest: fedora 16 with 32 vcpus 8GB memory.

From initial observations, at least for the ebizzy workload, the

63.56%     282095         qemu-kvm  [kernel.kallsyms]        [k] _raw_spin_lock                  
5.42%      24420         qemu-kvm  [kvm]                    [k] kvm_vcpu_yield_to               
5.33%      26481         qemu-kvm  [kernel.kallsyms]        [k] get_pid_task                    
4.35%      20049         qemu-kvm  [kernel.kallsyms]        [k] yield_to                        
2.74%      15652         qemu-kvm  [kvm]                    [k] kvm_apic_present                
1.70%       8657         qemu-kvm  [kvm]                    [k] kvm_vcpu_on_spin                
1.45%       7889         qemu-kvm  [kvm]                    [k] vcpu_enter_guest                

Quoted 3 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Hi, Raghu.

Quoted 29 lines

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

(a) Not done PLE exit at all (probably he is preempted lock-holder)

(b) VCPU skipped in last iteration because it did PL exit, and probably has become eligible now (next eligible lock holder)

Future enhancemnets:
(1) Currently we have a boolean to decide on eligibility of vcpu. It would be nice if I get feedback on guest (>32 vcpu) whether we can improve better with integer counter. (with counter = say f(log n )).

(2) We have not considered system load during iteration of vcpu. With that information we can limit the scan and also decide whether schedule() is better. [ I am able to use #kicked vcpus to decide on this But may be there are better ideas like information from global loadavg.]

(3) We can exploit this further with PV patches since it also knows about next eligible lock-holder.

Summary: There is a huge improvement for moderate / no overcommit scenario
for kvm based guest on PLE machine (which is difficult ;) ).

Quoted 5 lines

Result:
Base : kernel 3.5.0-rc5 with Rik's Ple handler fix

Machine : Intel(R) Xeon(R) CPU X7560 @ 2.27GHz, 4 numa node, 256GB RAM,
32 core machine

Is this with HT enabled, therefore 64 CPU threads?

Quoted 5 lines

Host: enterprise linux gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
with test kernels

Guest: fedora 16 with 32 vcpus 8GB memory.

Can you briefly explain the 1x and 2x configs? This of course is highly dependent whether or not HT is enabled...

FWIW, I started testing what I would call "0.5x", where I have one 40 vcpu guest running on a host with 40 cores and 80 CPU threads total (HT enabled, no extra load on the system). For ebizzy, the results are quite erratic from run to run, so I am inclined to discard it as a

workload, but maybe I should try "1x" and "2x" cpu over-commit as well.

Quoted 1 line

From initial observations, at least for the ebizzy workload, the

percentage of exits that result in a yield_to() are very low, around 1%, before these patches.

reducing that number even more has diminishing returns. I am however still concerned about the scalability problem with yield_to(),

shows like this for me (perf):

Quoted 7 lines

63.56%     282095         qemu-kvm  [kernel.kallsyms]        [k] _raw_spin_lock
5.42%      24420         qemu-kvm  [kvm]                    [k] kvm_vcpu_yield_to
5.33%      26481         qemu-kvm  [kernel.kallsyms]        [k] get_pid_task
4.35%      20049         qemu-kvm  [kernel.kallsyms]        [k] yield_to
2.74%      15652         qemu-kvm  [kvm]                    [k] kvm_apic_present
1.70%       8657         qemu-kvm  [kvm]                    [k] kvm_vcpu_on_spin
1.45%       7889         qemu-kvm  [kvm]                    [k] vcpu_enter_guest

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest.This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

Honestly, I not confident addressing this problem will improve the ebizzy score. That workload is so erratic for me, that I do not trust the results at all. I have however seen consistent improvements in disabling PLE for a http guest workload and a very high IOPS guest workload, both with much time spent in host in the double runqueue lock for yield_to(), so that's why I still gravitate toward that issue.

-Andrew Theurer

Quoted 3 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Honestly, I not confident addressing this problem will improve the ebizzy score. That workload is so erratic for me, that I do not trust the results at all. I have however seen consistent improvements in disabling PLE for a http guest workload and a very high IOPS guest workload, both with much time spent in host in the double runqueue lock for yield_to(), so that's why I still gravitate toward that issue.

Quoted 3 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Hi, Raghu.

Can you briefly explain the 1x and 2x configs? This of course is highly dependent whether or not HT is enabled...

Quoted 7 lines · Andrew Theurer

Quoted 3 lines · Raghavendra K T

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Hi, Raghu.

[...]

Quoted 4 lines

Can you briefly explain the 1x and 2x configs? This of course is highly dependent whether or not HT is enabled...

Sorry if I had not made very clear in earlier threads. Have you applied Rik's following patch for base. without this you could see some inconsistent results perhaps.

https://lkml.org/lkml/2012/6/19/401

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest. This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

Quoted 7 lines · Andrew Theurer

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest. This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

One thing we can do is autotune ple_window. If a ple exit fails to wake anybody (because all vcpus are either running, sleeping, or in ple exits) then we deduce we are not overcommitted and we can increase the ple window. There's the question of how to decrease it again though.

Quoted 9 lines · Avi Kivity

Quoted 7 lines · Andrew Theurer

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest. This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

Quoted 5 lines

One thing we can do is autotune ple_window. If a ple exit fails to wake anybody (because all vcpus are either running, sleeping, or in ple exits) then we deduce we are not overcommitted and we can increase the ple window. There's the question of how to decrease it again though.

I see some problem here, If I interpret situation correctly. What happens if we have two guests with one VM having no over-commit and other with high over-commit. (except when we have gang scheduling).

Rather we should have something tied to VM rather than rigid PLE window.

Quoted 16 lines · Raghavendra K T

Quoted 14 lines · Avi Kivity

Quoted 7 lines · Andrew Theurer

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest. This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

One thing we can do is autotune ple_window. If a ple exit fails to wake anybody (because all vcpus are either running, sleeping, or in ple exits) then we deduce we are not overcommitted and we can increase the ple window. There's the question of how to decrease it again though.

Quoted 4 lines

I see some problem here, If I interpret situation correctly. What happens if we have two guests with one VM having no over-commit and other with high over-commit. (except when we have gang scheduling).

Sorry, I meant less load and high load inside the guest.

Quoted 2 lines

Rather we should have something tied to VM rather than rigid PLE window.

Quoted 21 lines · Raghavendra K T

Quoted 20 lines · Raghavendra K T

Quoted 9 lines · Avi Kivity

Quoted 7 lines · Andrew Theurer

For the cpu threads in the host that are actually active (in this case 1/2 of them), ~50% of their time is in kernel and ~43% in guest. This is for a no-IO workload, so that's just incredible to see so much cpu wasted. I feel that 2 important areas to tackle are a more scalable yield_to() and reducing the number of pause exits itself (hopefully by just tuning ple_window for the latter).

Quoted 5 lines

One thing we can do is autotune ple_window. If a ple exit fails to wake anybody (because all vcpus are either running, sleeping, or in ple exits) then we deduce we are not overcommitted and we can increase the ple window. There's the question of how to decrease it again though.

I see some problem here, If I interpret situation correctly. What happens if we have two guests with one VM having no over-commit and other with high over-commit. (except when we have gang scheduling).

Quoted 4 lines

Sorry, I meant less load and high load inside the guest.

Quoted 2 lines

Rather we should have something tied to VM rather than rigid PLE window.

The problem occurs even with no overcommit at all. One vcpu is in a legitimately long pause loop. All those exits accomplish nothing, since all vcpus are scheduled. Better to let it spin in guest mode.

Quoted 5 lines

The problem occurs even with no overcommit at all. One vcpu is in a legitimately long pause loop. All those exits accomplish nothing, since all vcpus are scheduled. Better to let it spin in guest mode.

I agree. One idea is we can have a scan_window to limit the scan of all n vcpus each time we enter vcpu_spin, to say 2*log n initially;

Currently Pause Looop Exit (PLE) handler is doing directed yield to a random VCPU on PL exit. Though we already have filtering while choosing the candidate to yield_to, we can do better.

Problem is, for large vcpu guests, we have more probability of yielding to a bad vcpu. We are not able to prevent directed yield to same guy who has done PL exit recently, who perhaps spins again and wastes CPU.

Fix that by keeping track of who has done PL exit. So The Algorithm in series give chance to a VCPU which has:

(a) Not done PLE exit at all (probably he is preempted lock-holder)

(b) VCPU skipped in last iteration because it did PL exit, and probably has become eligible now (next eligible lock holder)

Future enhancemnets: