[v3 0/3] KVM: irqchip: synchronize srcu only if needed

[v3 0/3] KVM: irqchip: synchronize srcu only if needed

[Yi Wang]

From: Yi Wang <foxywang@tencent.com>

We found that it may cost more than 20 milliseconds very accidentally
to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
already.

The reason is that when vmm(qemu/CloudHypervisor) invokes
KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
might_sleep and kworker of srcu may cost some delay during this period.
One way makes sence is setup empty irq routing when creating vm and
so that x86/s390 don't need to setup empty/dummy irq routing.

Note: I have no s390 machine so the s390 patch has not been tested.

Changelog:
----------
v3:
  - squash setup empty routing function and use of that into one commit
  - drop the comment in s390 part

v2:
  - setup empty irq routing in kvm_create_vm
  - don't setup irq routing in x86 KVM_CAP_SPLIT_IRQCHIP
  - don't setup irq routing in s390 KVM_CREATE_IRQCHIP

v1: https://lore.kernel.org/kvm/20240112091128.3868059-1-foxywang@tencent.com/

Yi Wang (3):
  KVM: setup empty irq routing when create vm
  KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP
  KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP

 arch/s390/kvm/kvm-s390.c |  9 +--------
 arch/x86/kvm/irq.h       |  1 -
 arch/x86/kvm/irq_comm.c  |  5 -----
 arch/x86/kvm/x86.c       |  3 ---
 include/linux/kvm_host.h |  1 +
 virt/kvm/irqchip.c       | 19 +++++++++++++++++++
 virt/kvm/kvm_main.c      |  4 ++++
 7 files changed, 25 insertions(+), 17 deletions(-)

-- 
2.39.3

[v3 1/3] KVM: setup empty irq routing when create vm

[Yi Wang]

From: Yi Wang <foxywang@tencent.com>

Add a new function to setup empty irq routing in kvm path, which
can be invoded in non-architecture-specific functions. The difference
compared to the kvm_setup_empty_irq_routing() is this function just
alloc the empty irq routing and does not need synchronize srcu, as
we will call it in kvm_create_vm().

Using the new adding function, we can setup empty irq routing when
kvm_create_vm(), so that x86 and s390 no longer need to set
empty/dummy irq routing when creating an IRQCHIP 'cause it avoid
an synchronize_srcu.

Signed-off-by: Yi Wang <foxywang@tencent.com>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/irqchip.c       | 19 +++++++++++++++++++
 virt/kvm/kvm_main.c      |  4 ++++
 3 files changed, 24 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4944136efaa2..e91525c0a4ea 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2000,6 +2000,7 @@ int kvm_set_irq_routing(struct kvm *kvm,
 			const struct kvm_irq_routing_entry *entries,
 			unsigned nr,
 			unsigned flags);
+int kvm_setup_empty_irq_routing_lockless(struct kvm *kvm);
 int kvm_set_routing_entry(struct kvm *kvm,
 			  struct kvm_kernel_irq_routing_entry *e,
 			  const struct kvm_irq_routing_entry *ue);
diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 1e567d1f6d3d..90fc43bd0fe4 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -237,3 +237,22 @@ int kvm_set_irq_routing(struct kvm *kvm,
 
 	return r;
 }
+
+int kvm_setup_empty_irq_routing_lockless(struct kvm *kvm)
+{
+	struct kvm_irq_routing_table *new;
+	u32 i, j;
+
+	new = kzalloc(struct_size(new, map, 1), GFP_KERNEL_ACCOUNT);
+	if (!new)
+		return -ENOMEM;
+
+	new->nr_rt_entries = 1;
+	for (i = 0; i < KVM_NR_IRQCHIPS; i++)
+		for (j = 0; j < KVM_IRQCHIP_NUM_PINS; j++)
+			new->chip[i][j] = -1;
+
+	RCU_INIT_POINTER(kvm->irq_routing, new);
+
+	return 0;
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7db96875ac46..db1b13fc0502 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1242,6 +1242,10 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 	if (r)
 		goto out_err;
 
+	r = kvm_setup_empty_irq_routing_lockless(kvm);
+	if (r)
+		goto out_err;
+
 	mutex_lock(&kvm_lock);
 	list_add(&kvm->vm_list, &vm_list);
 	mutex_unlock(&kvm_lock);
-- 
2.39.3

[v3 2/3] KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP

[Yi Wang]

From: Yi Wang <foxywang@tencent.com>

We found that it may cost more than 20 milliseconds very accidentally
to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
already.

The reason is that when vmm(qemu/CloudHypervisor) invokes
KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
might_sleep and kworker of srcu may cost some delay during this period.

As we have set up empty irq routing when creating vm, so this is no
need now.

Signed-off-by: Yi Wang <foxywang@tencent.com>
---
 arch/x86/kvm/irq.h      | 1 -
 arch/x86/kvm/irq_comm.c | 5 -----
 arch/x86/kvm/x86.c      | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index c2d7cfe82d00..76d46b2f41dd 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -106,7 +106,6 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu);
 int apic_has_pending_timer(struct kvm_vcpu *vcpu);
 
 int kvm_setup_default_irq_routing(struct kvm *kvm);
-int kvm_setup_empty_irq_routing(struct kvm *kvm);
 int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 			     struct kvm_lapic_irq *irq,
 			     struct dest_map *dest_map);
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 16d076a1b91a..99bf53b94175 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -392,11 +392,6 @@ int kvm_setup_default_irq_routing(struct kvm *kvm)
 
 static const struct kvm_irq_routing_entry empty_routing[] = {};
 
-int kvm_setup_empty_irq_routing(struct kvm *kvm)
-{
-	return kvm_set_irq_routing(kvm, empty_routing, 0, 0);
-}
-
 void kvm_arch_post_irq_routing_update(struct kvm *kvm)
 {
 	if (!irqchip_split(kvm))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cec0fc2a4b1c..6a2e786aca22 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6414,9 +6414,6 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 			goto split_irqchip_unlock;
 		if (kvm->created_vcpus)
 			goto split_irqchip_unlock;
-		r = kvm_setup_empty_irq_routing(kvm);
-		if (r)
-			goto split_irqchip_unlock;
 		/* Pairs with irqchip_in_kernel. */
 		smp_wmb();
 		kvm->arch.irqchip_mode = KVM_IRQCHIP_SPLIT;
-- 
2.39.3

[v3 3/3] KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP

[Yi Wang]

From: Yi Wang <foxywang@tencent.com>

As we have setup empty irq routing in kvm_create_vm(), there's
no need to setup dummy routing when KVM_CREATE_IRQCHIP.

Signed-off-by: Yi Wang <foxywang@tencent.com>
---
 arch/s390/kvm/kvm-s390.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index acc81ca6492e..dec3c026a6c1 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2999,14 +2999,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
 		break;
 	}
 	case KVM_CREATE_IRQCHIP: {
-		struct kvm_irq_routing_entry routing;
-
-		r = -EINVAL;
-		if (kvm->arch.use_irqchip) {
-			/* Set up dummy routing. */
-			memset(&routing, 0, sizeof(routing));
-			r = kvm_set_irq_routing(kvm, &routing, 0, 0);
-		}
+		r = 0;
 		break;
 	}
 	case KVM_SET_DEVICE_ATTR: {
-- 
2.39.3

Re: [v3 0/3] KVM: irqchip: synchronize srcu only if needed

[Yi Wang]

Gentle ping :)

On Wed, Jan 24, 2024 at 7:35 PM Yi Wang <up2wing@gmail.com> wrote:
>
> From: Yi Wang <foxywang@tencent.com>
>
> We found that it may cost more than 20 milliseconds very accidentally
> to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
> already.
>
> The reason is that when vmm(qemu/CloudHypervisor) invokes
> KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
> might_sleep and kworker of srcu may cost some delay during this period.
> One way makes sence is setup empty irq routing when creating vm and
> so that x86/s390 don't need to setup empty/dummy irq routing.
>
> Note: I have no s390 machine so the s390 patch has not been tested.
>
> Changelog:
> ----------
> v3:
>   - squash setup empty routing function and use of that into one commit
>   - drop the comment in s390 part
>
> v2:
>   - setup empty irq routing in kvm_create_vm
>   - don't setup irq routing in x86 KVM_CAP_SPLIT_IRQCHIP
>   - don't setup irq routing in s390 KVM_CREATE_IRQCHIP
>
> v1: https://lore.kernel.org/kvm/20240112091128.3868059-1-foxywang@tencent.com/
>
> Yi Wang (3):
>   KVM: setup empty irq routing when create vm
>   KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP
>   KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP
>
>  arch/s390/kvm/kvm-s390.c |  9 +--------
>  arch/x86/kvm/irq.h       |  1 -
>  arch/x86/kvm/irq_comm.c  |  5 -----
>  arch/x86/kvm/x86.c       |  3 ---
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/irqchip.c       | 19 +++++++++++++++++++
>  virt/kvm/kvm_main.c      |  4 ++++
>  7 files changed, 25 insertions(+), 17 deletions(-)
>
> --
> 2.39.3
>


-- 
---
Best wishes
Yi Wang

Re: [v3 0/3] KVM: irqchip: synchronize srcu only if needed

[Dongli Zhang]

Hi Yi,

On 1/24/24 03:34, Yi Wang wrote:
> From: Yi Wang <foxywang@tencent.com>
> 
> We found that it may cost more than 20 milliseconds very accidentally
> to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
> already.

Would you mind explaining the reason that the *number of VMs* matters, as
KVM_CAP_SPLIT_IRQCHIP is a per-VM cap?

Or it meant it is more likely to have some VM workload impacted by the
synchronize_srcu_expedited() as in prior discussion?

https://lore.kernel.org/kvm/CAN35MuSkQf0XmBZ5ZXGhcpUCGD-kKoyTv9G7ya4QVD1xiqOxLg@mail.gmail.com/

Thank you very much!

Dongli Zhang

> 
> The reason is that when vmm(qemu/CloudHypervisor) invokes
> KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
> might_sleep and kworker of srcu may cost some delay during this period.
> One way makes sence is setup empty irq routing when creating vm and
> so that x86/s390 don't need to setup empty/dummy irq routing.
> 
> Note: I have no s390 machine so the s390 patch has not been tested.
> 
> Changelog:
> ----------
> v3:
>   - squash setup empty routing function and use of that into one commit
>   - drop the comment in s390 part
> 
> v2:
>   - setup empty irq routing in kvm_create_vm
>   - don't setup irq routing in x86 KVM_CAP_SPLIT_IRQCHIP
>   - don't setup irq routing in s390 KVM_CREATE_IRQCHIP
> 
> v1: https://urldefense.com/v3/__https://lore.kernel.org/kvm/20240112091128.3868059-1-foxywang@tencent.com/__;!!ACWV5N9M2RV99hQ!LjwKfBaGVl3u1l9YQSskg_1RU6278h2-fYnYLsoihF9i43aq73eIDqolGzOmeRvO8UlPreQHLqXEL1bAuw$ 
> 
> Yi Wang (3):
>   KVM: setup empty irq routing when create vm
>   KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP
>   KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP
> 
>  arch/s390/kvm/kvm-s390.c |  9 +--------
>  arch/x86/kvm/irq.h       |  1 -
>  arch/x86/kvm/irq_comm.c  |  5 -----
>  arch/x86/kvm/x86.c       |  3 ---
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/irqchip.c       | 19 +++++++++++++++++++
>  virt/kvm/kvm_main.c      |  4 ++++
>  7 files changed, 25 insertions(+), 17 deletions(-)
> 

Re: [v3 0/3] KVM: irqchip: synchronize srcu only if needed

[Yi Wang]

Hi Dongli,

Thanks for the reply and Happy Spring Festival to all :)

On Fri, Feb 9, 2024 at 5:00 PM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>
> Hi Yi,
>
> On 1/24/24 03:34, Yi Wang wrote:
> > From: Yi Wang <foxywang@tencent.com>
> >
> > We found that it may cost more than 20 milliseconds very accidentally
> > to enable cap of KVM_CAP_SPLIT_IRQCHIP on a host which has many vms
> > already.
>
> Would you mind explaining the reason that the *number of VMs* matters, as
> KVM_CAP_SPLIT_IRQCHIP is a per-VM cap?
>
> Or it meant it is more likely to have some VM workload impacted by the
> synchronize_srcu_expedited() as in prior discussion?
>
> https://lore.kernel.org/kvm/CAN35MuSkQf0XmBZ5ZXGhcpUCGD-kKoyTv9G7ya4QVD1xiqOxLg@mail.gmail.com/
>

The actual reason is might_sleep() and the kworker in
synchronize_srcu_expedited(),
which may cause some delay when there are pretty many threads in the host, so
"number of VMs" is just one of  the situations which can trigger the issue :)

> Thank you very much!
>
> Dongli Zhang
>
> >
> > The reason is that when vmm(qemu/CloudHypervisor) invokes
> > KVM_CAP_SPLIT_IRQCHIP kvm will call synchronize_srcu_expedited() and
> > might_sleep and kworker of srcu may cost some delay during this period.
> > One way makes sence is setup empty irq routing when creating vm and
> > so that x86/s390 don't need to setup empty/dummy irq routing.
> >
> > Note: I have no s390 machine so the s390 patch has not been tested.
> >
> > Changelog:
> > ----------
> > v3:
> >   - squash setup empty routing function and use of that into one commit
> >   - drop the comment in s390 part
> >
> > v2:
> >   - setup empty irq routing in kvm_create_vm
> >   - don't setup irq routing in x86 KVM_CAP_SPLIT_IRQCHIP
> >   - don't setup irq routing in s390 KVM_CREATE_IRQCHIP
> >
> > v1: https://urldefense.com/v3/__https://lore.kernel.org/kvm/20240112091128.3868059-1-foxywang@tencent.com/__;!!ACWV5N9M2RV99hQ!LjwKfBaGVl3u1l9YQSskg_1RU6278h2-fYnYLsoihF9i43aq73eIDqolGzOmeRvO8UlPreQHLqXEL1bAuw$
> >
> > Yi Wang (3):
> >   KVM: setup empty irq routing when create vm
> >   KVM: x86: don't setup empty irq routing when KVM_CAP_SPLIT_IRQCHIP
> >   KVM: s390: don't setup dummy routing when KVM_CREATE_IRQCHIP
> >
> >  arch/s390/kvm/kvm-s390.c |  9 +--------
> >  arch/x86/kvm/irq.h       |  1 -
> >  arch/x86/kvm/irq_comm.c  |  5 -----
> >  arch/x86/kvm/x86.c       |  3 ---
> >  include/linux/kvm_host.h |  1 +
> >  virt/kvm/irqchip.c       | 19 +++++++++++++++++++
> >  virt/kvm/kvm_main.c      |  4 ++++
> >  7 files changed, 25 insertions(+), 17 deletions(-)
> >



-- 
---
Best wishes
Yi Wang