On Wed, Apr 21, 2010 at 03:29:11PM -0300, Marcelo Tosatti wrote: > On Wed, Apr 21, 2010 at 08:58:48PM +0300, Gleb Natapov wrote: > > On Wed, Apr 21, 2010 at 02:37:34PM -0300, Marcelo Tosatti wrote: > > > On Wed, Apr 21, 2010 at 08:12:27PM +0300, Gleb Natapov wrote: > > > > On Wed, Apr 21, 2010 at 12:58:41PM -0300, Marcelo Tosatti wrote: > > > > > > Or could we make kvm_set_irq() atomic? Though the code path is a little long > > > > > > for spinlock. > > > > > > > > > > Yes, given the sleep-inside-RCU-protected section bug from > > > > > kvm_notify_acked_irq, either that or convert IRQ locking to SRCU. > > > > > > > > > > But as you said, the code paths are long and potentially slow, so > > > > > probably SRCU is a better alternative. > > > > > > > > > > Gleb? > > > > kvm_set_irq() was converted to rcu from mutex to make msix interrupt > > > > injection scalable. > > > > > > We meant ioapic lock. See the last report from Ralf on this thread. > > Can we solve the problem by calling ack notifier outside rcu read > > section in kvm_notify_acked_irq()? > > The unregister path does > > - remove_from_list(entry) > - synchronize_rcu > - kfree(entry) > > So if kvm_notify_acked_irq sleeps, synchronize_rcu can succeed, and the > notifier entry can be freed. What I mean is kvm_notify_acked_irq() will iterate over all ack entries in rcu read protected section, but instead of calling callback it will collect them into the array and call them outside rcu read section. At this point it doesn't matter if entry is unregistered since the copy is used to actually call the notifier. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
