Re: [PATCH] kvm: remove in_range from kvm_io_device

[Gregory Haskins]

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,
-Greg

Attachment: signature.asc
Description: OpenPGP digital signature