Re: [PATCH] kvm: remove in_range from kvm_io_device

Gregory Haskins
2009-06-25T15:45:44Z

[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]

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?
>

I just took another look. Things are quite messy and I don't know what
the optimal order of merging is. Go ahead with your remove-inrange and
srcu plan and I will just defer submitting the iosignalfd related series
till you are happy with the new layout.

Thanks Michael,