lkml.org 
[lkml]   [2012]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v9 0/2] kvm: level irqfd support
    From
    Date
    On Wed, 2012-08-22 at 11:25 +0300, Michael S. Tsirkin wrote:
    > On Tue, Aug 21, 2012 at 07:28:15PM -0600, Alex Williamson wrote:
    > > On Wed, 2012-08-22 at 03:31 +0300, Michael S. Tsirkin wrote:
    > > > On Tue, Aug 21, 2012 at 01:28:57PM -0600, Alex Williamson wrote:
    > > > > Here's the much anticipated re-write of support for level irqfds. As
    > > > > Michael suggested, I've rolled the eoi/ack notification fd into
    > > > > KVM_IRQFD as a new mode. For lack of a better name, as there seems to
    > > > > be objections to associating this specifically with an EOI or an ACK,
    > > > > I've name this OADN or "On Ack, De-assert & Notify".
    > > > >
    > > > > Patch 1of2 switches current KVM_IRQFDs to use their own IRQ source ID
    > > > > since we're potentially stepping on KVM_USERSPACE_IRQ_SOURCE_ID.
    > > > > Unfurtunately I was not able to make 2of2 use a single IRQ source ID,
    > > > > the reason is it's racy. Objects to track OADNs are made dynamically,
    > > > > we look through existing ones for a match under spinlock and setup a
    > > > > new one if there's no match. On teardown, we can remove the OADN from
    > > > > the list under lock, but that same lock prevents us from de-assigning
    > > > > the IRQ ACK notifier or waiting for an RCU grace period. We must make
    > > > > sure that any unused GSI is de-asserted, but the above means it's
    > > > > possible that another OADN has been created for this source ID/GSI
    > > > > and de-asserting the GSI could lead to breakage.
    > > >
    > > > I do not see it. What breakage? Could you give an example please?
    > > >
    > > >
    > > > I think what you are saying is last deassign must clear
    > > > since otherwise we never will clear.
    > > > I agree it is either that or delay deassign until ack.
    > > >
    > > > Can it be as simple as this (after all rcu etc dances)?
    > > > lock irqfds
    > > > if no oadns
    > > > set level to 0
    > > > unlock irqfds
    > > > ?
    > >
    > > lock irqfds
    > > remove irqfd from oadn list
    > > if no oadns
    > > remove oadn
    > > set gsi 0
    > > unlock
    > > lock irqfds
    > > new oadn
    > > unlock irqfds
    > >
    > > >> EOI
    > > ack notify new oadn
    > > de-assert gsi
    > > notify new oadn
    > > >> re-assert irqfd
    > > ack notify old oadn
    > > de-assert gsi
    > > notify old oadn
    > >
    > > synchronize_rcu
    > >
    > > kvm_unregister_irq_ack_notifier
    > >
    > > So, because the unregister is removed from the final clear and because
    > > we share an IRQ source ID there's a window where we can have two oadns
    > > registered for the same GSI. The new one will de-assert and notify
    > > while the old one has an empty list to notify, but still de-asserts. We
    > > can therefore de-assert w/o notify.
    > >
    > > By using a new source ID, we separate the two so users of the new oadn
    > > can't race the old and we can cleanly free the old source ID,
    > > de-asserting it.
    >
    > Need to think about it some more but is the problem two
    > ack notifiers for the same gsi?

    yes

    > In that case, how about we add __kvm_unregister_irq_ack_notifier
    > with no locking, and do most of the above under
    > kvm->irq_lock?

    Converting locks makes me nervous, but I'll give it a shot. I don't
    know how easy/possible it is though. I know in previous iterations I
    tried to make something similar to irqfd use a mutex and couldn't, but I
    don't remember the details.

    > With one change: it is better not to call synchronize_rcu
    > under irq lock, I think we can safely move it to after
    > __kvm_unregister_irq_ack_notifier.

    Yep, that makes the interface pretty ugly though as we then have two
    separate, but dependent steps. Thanks,

    Alex



    \
     
     \ /
      Last update: 2012-09-17 20:41    [W:2.259 / U:0.504 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site