Re: [PATCH 3/4] kvm: Extend irqfd to support level interrupts

[Alex Williamson]

On Sun, 2012-06-24 at 18:49 +0300, Michael S. Tsirkin wrote:
> On Sun, Jun 24, 2012 at 09:18:38AM -0600, Alex Williamson wrote:
> > > > @@ -242,7 +299,8 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
> > > >  
> > > >  	ret = 0;
> > > >  	list_for_each_entry(tmp, &kvm->irqfds.items, list) {
> > > > -		if (irqfd->eventfd != tmp->eventfd)
> > > > +		if (irqfd->eventfd != tmp->eventfd &&
> > > > +		    irqfd->eventfd != tmp->eoi_eventfd)
> > > >  			continue;
> > > 
> > > So we allow duplicate irqfd with differing eoifd (or edge-triggered and
> > > level-triggered irqfd on the same context).
> > > 
> > > (why the check in the first place? just so we can have a reliable
> > > deassign or is it avoiding a deeper problem?)
> > 
> > I really wasn't sure to what extent we wanted to prevent duplicates.  My
> > guess was that we don't want to have an irqfd trigger more than one
> > thing.  That seems to be what the current code does.  I don't see any
> > problems with multiple irqfds triggering the same eventfd though.  I
> > only added a test that a new irqfd can't be triggered by an existing
> > eoi_eventfd as that could make a nasty loop.
> 
> How would that make a loop? You can have the same thing
> with e.g. ioeventfd - why isn't it a problem there?

eoi_eventfd1 -> irqfd2 [eoi] eoi_eventfd2 -> irqfd1 [eoi] eoi_eventfd1 ->...

Yes, in reality we'd need to search fds from all the interfaces and come
up with some grossly complicated truth table of what's allowed and
what's not.  The original code didn't go to that kind of extreme, so I
just added something that seemed like a reasonable case
we'd want to prevent.  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/