Re: [RFC PATCH] VFIO: Add a parameter to force nonthread IRQ

From: Alex Williamson
Date: Wed Oct 28 2015 - 12:00:38 EST


On Wed, 2015-10-28 at 01:44 +0100, Paolo Bonzini wrote:
>
> On 27/10/2015 22:26, Yunhong Jiang wrote:
> >> > On RT kernels however can you call eventfd_signal from interrupt
> >> > context? You cannot call spin_lock_irqsave (which can sleep) from a
> >> > non-threaded interrupt handler, can you? You would need a raw spin lock.
> > Thanks for pointing this out. Yes, we can't call spin_lock_irqsave on RT
> > kernel. Will do this way on next patch. But not sure if it's overkill to use
> > raw_spinlock there since the eventfd_signal is used by other caller also.
>
> No, I don't think you can use raw_spinlock there. The problem is not
> just eventfd_signal, it is especially wake_up_locked_poll. You cannot
> convert the whole workqueue infrastructure to use raw_spinlock.
>
> Alex, would it make sense to use the IRQ bypass infrastructure always,
> not just for VT-d, to do the MSI injection directly from the VFIO
> interrupt handler and bypass the eventfd? Basically this would add an
> RCU-protected list of consumers matching the token to struct
> irq_bypass_producer, and a
>
> int (*inject)(struct irq_bypass_consumer *);
>
> callback to struct irq_bypass_consumer. If any callback returns true,
> the eventfd is not signaled. The KVM implementation would be like this
> (compare with virt/kvm/eventfd.c):
>
> /* Extracted out of irqfd_wakeup */
> static int
> irqfd_wakeup_pollin(struct kvm_kernel_irqfd *irqfd)
> {
> ...
> }
>
> /* Extracted out of irqfd_wakeup */
> static int
> irqfd_wakeup_pollhup(struct kvm_kernel_irqfd *irqfd)
> {
> ...
> }
>
> static int
> irqfd_wakeup(wait_queue_t *wait, unsigned mode, int sync,
> void *key)
> {
> struct _irqfd *irqfd = container_of(wait,
> struct _irqfd, wait);
> unsigned long flags = (unsigned long)key;
>
> if (flags & POLLIN)
> irqfd_wakeup_pollin(irqfd);
> if (flags & POLLHUP)
> irqfd_wakeup_pollhup(irqfd);
>
> return 0;
> }
>
> static int kvm_arch_irq_bypass_inject(
> struct irq_bypass_consumer *cons)
> {
> struct kvm_kernel_irqfd *irqfd =
> container_of(cons, struct kvm_kernel_irqfd,
> consumer);
>
> irqfd_wakeup_pollin(irqfd);
> }
>
> Or do you think it would be a hack? The latency improvement might
> actually be even better than what Yunhong is already reporting.

Yeah, that might be a good idea, it's probably more plausible than
making the eventfd_signal() code friendly to call from hard interrupt
context. On the vfio side can we use request_threaded_irq() directly
for this? Making the hard irq handler return IRQ_HANDLED if we can use
the irq bypass manager or IRQ_WAKE_THREAD if we need to use the eventfd.
I think we need some way to get back to irq thread context to use
eventfd_signal(). Would we ever not want to use the direct bypass
manager path if available? Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/