All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Yunhong Jiang <yunhong.jiang@linux.intel.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] VFIO: Add a parameter to force nonthread IRQ
Date: Mon, 26 Oct 2015 21:37:14 -0600	[thread overview]
Message-ID: <1445917034.8018.220.camel@redhat.com> (raw)
In-Reply-To: <1445908801-14732-1-git-send-email-yunhong.jiang@linux.intel.com>

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 <yunhong.jiang@linux.intel.com>
> ---
>  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 <linux/vfio.h>
>  #include <linux/wait.h>
>  #include <linux/slab.h>
> +#include <linux/module.h>
>  
>  #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);




  reply	other threads:[~2015-10-27  3:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-27  1:20 [RFC PATCH] VFIO: Add a parameter to force nonthread IRQ Yunhong Jiang
2015-10-27  3:37 ` Alex Williamson [this message]
2015-10-27  6:35   ` Yunhong Jiang
2015-10-27  9:29     ` Paolo Bonzini
2015-10-27 21:26       ` Yunhong Jiang
2015-10-28  0:44         ` Paolo Bonzini
2015-10-28 16:00           ` Alex Williamson
2015-10-28 17:05             ` Paolo Bonzini
2015-10-28 23:54               ` Marcelo Tosatti
2015-10-29  3:11               ` Alex Williamson
2015-10-29  9:45                 ` Paolo Bonzini
2015-10-30  6:16                   ` Yunhong Jiang
2015-11-02  9:17                     ` Paolo Bonzini
2015-10-28 17:50           ` Yunhong Jiang
2015-10-28 18:18             ` Alex Williamson
2015-10-28 21:46               ` Yunhong Jiang
2015-10-28 18:28             ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1445917034.8018.220.camel@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yunhong.jiang@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.