From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752830AbbJ0DhR (ORCPT ); Mon, 26 Oct 2015 23:37:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46974 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751572AbbJ0DhP (ORCPT ); Mon, 26 Oct 2015 23:37:15 -0400 Message-ID: <1445917034.8018.220.camel@redhat.com> Subject: Re: [RFC PATCH] VFIO: Add a parameter to force nonthread IRQ From: Alex Williamson To: Yunhong Jiang Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org Date: Mon, 26 Oct 2015 21:37:14 -0600 In-Reply-To: <1445908801-14732-1-git-send-email-yunhong.jiang@linux.intel.com> References: <1445908801-14732-1-git-send-email-yunhong.jiang@linux.intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2015-10-26 at 18:20 -0700, Yunhong Jiang wrote: > An option to force VFIO PCI MSI/MSI-X handler as non-threaded IRQ, > even when CONFIG_IRQ_FORCED_THREADING=y. This is uselful when > assigning a device to a guest with low latency requirement since it > reduce the context switch to/from the IRQ thread. Is there any way we can do this automatically? Perhaps detecting that we're on a RT kernel or maybe that the user is running with RT priority? I find that module options are mostly misunderstood and misused. > An experiment was conducted on a HSW platform for 1 minutes, with the > guest vCPU bound to isolated pCPU. The assigned device triggered the > interrupt every 1ms. The average EXTERNAL_INTERRUPT exit handling time > is dropped from 5.3us to 2.2us. > > Another choice is to change VFIO_DEVICE_SET_IRQS ioctl, to apply this > option only to specific devices when in kernel irq_chip is enabled. It > provides more flexibility but is more complex, not sure if we need go > through that way. Allowing the user to decide whether or not to use a threaded IRQ seems like a privilege violation; a chance for the user to game the system and give themselves better latency, maybe at the cost of others. I think we're better off trying to infer the privilege from the task priority or kernel config or, if we run out of options, make a module option as you have here requiring the system admin to provide the privilege. Thanks, Alex > Signed-off-by: Yunhong Jiang > --- > drivers/vfio/pci/vfio_pci_intrs.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c > index 1f577b4..ca1f95a 100644 > --- a/drivers/vfio/pci/vfio_pci_intrs.c > +++ b/drivers/vfio/pci/vfio_pci_intrs.c > @@ -22,9 +22,13 @@ > #include > #include > #include > +#include > > #include "vfio_pci_private.h" > > +static bool nonthread_msi = 1; > +module_param(nonthread_msi, bool, 0444); > + > /* > * INTx > */ > @@ -313,6 +317,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev, > char *name = msix ? "vfio-msix" : "vfio-msi"; > struct eventfd_ctx *trigger; > int ret; > + unsigned long irqflags = 0; > > if (vector >= vdev->num_ctx) > return -EINVAL; > @@ -352,7 +357,10 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev, > pci_write_msi_msg(irq, &msg); > } > > - ret = request_irq(irq, vfio_msihandler, 0, > + if (nonthread_msi) > + irqflags = IRQF_NO_THREAD; > + > + ret = request_irq(irq, vfio_msihandler, irqflags, > vdev->ctx[vector].name, trigger); > if (ret) { > kfree(vdev->ctx[vector].name);