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

[Alex Williamson <alex.williamson@redhat.com>]

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