On Thu, Jun 25, 2009 at 07:27:36AM -0400, Gregory Haskins wrote: > Michael S. Tsirkin wrote: > > On Wed, Jun 24, 2009 at 11:49:01AM +0300, Michael S. Tsirkin wrote: > > > >> On Tue, Jun 23, 2009 at 09:43:31PM -0400, Gregory Haskins wrote: > >> > >>> Michael S. Tsirkin wrote: > >>> > >>>> Remove in_range from kvm_io_device and ask read/write callbacks, if > >>>> supplied, to perform range checks internally. This allows aliasing > >>>> (mostly for in-kernel virtio), as well as better error handling by > >>>> making it possible to pass errors up to userspace. And it's enough to > >>>> look at the diffstat to see that it's a better API anyway. > >>>> > >>>> While we are at it, document locking rules for kvm_io_device. > >>>> > >>>> > >>> Hi Michael, > >>> > >>> I just tried to apply this to kvm.git/master, and it blew up really > >>> badly. What tree should I be using? > >>> > >> Ugh, this is against 2.6.30. I'll post kvm.git version soon. > >> > >> > > > > I went ahead and tried to rebase it, to find that it conflicts with > > recent patch 35b3038961f94e83557944ae0d30c8fa0b5012cf merged in kvm.git: > > 'KVM: switch irq injection/acking data structures to irq_lock' > > which now does this: > > lock > > find > > unlock > > write > > > > I thought for a while that it might make sense to start small and just > > add in_range parameter for starters ... > > However, I just realised that this only works because devices are not > > added or removed dynamically. > > > > The long term fix is to switch to SRCU for bus management. But if we > > need to do this for iosignalfd anyway, in_range removal becomes possible > > again. > > > > Short term it might be also possible to go back to keeping kvm lock > > across both find and read - since the lock is taken, we don't > > really win anything currently if we drop the lock earlier. > > > > Comments? > > > > > > Can we just get iosignalfd merged for now as it is, then? The patch that adds value to in_range is still okay. We could go that way for now. But what I am saying is that groups are still devices, and the patch that adds kvm_io_bus_unregister_dev seems broken with the new locking. No? > Its not like > we cant patch the group/item code later to clean it up when the time is > right. > > Regards, > -Greg > What's the rush? IMO the best plan is: - add srcu for io bus, removing kvm lock from that space completely - apply in_range removal patch on top of this - iosignalfd on top Comments? -- MST -- 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
