Re: [PATCH 3/3] kvm-s390: streamline memslot handling - rebased

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Jun 08, 2009 at 12:51:26PM +0200, Christian Ehrhardt wrote:
>>>  Index: kvm/virt/kvm/kvm_main.c
>>> ===================================================================
>>> --- kvm.orig/virt/kvm/kvm_main.c
>>> +++ kvm/virt/kvm/kvm_main.c
>>> @@ -1682,6 +1682,10 @@ static int kvm_vcpu_release(struct inode
>>>  {
>>>  	struct kvm_vcpu *vcpu = filp->private_data;
>>>  +	clear_bit(KVM_REQ_MMU_RELOAD, &vcpu->requests);
>>> +	smp_mb__after_clear_bit();
>>> +	wake_up_bit(&vcpu->requests, KVM_REQ_MMU_RELOAD);
>>> +
>>>     
>>
>> And this should be generic? Say if other architectures want to make use 
>> of a similar wait infrastructure. Talk is cheap.
>>   
> Clear bit and wake up on release doesn't hurt any architecture, but it  
> is at a good place fine for those using the mechanism to ensure cleaning  
> up outstanding things when closing a vcpu fd.
> I thought its not worth to add kvm_ARCH_vcpu_release for it while I  
> could do so if we want it separated.

Yeah, was frustated for lack of more useful comments so decided to
nitpick on something.

> (continued below)
>> Anyway, yeah, the set request / wait mechanism you implement here is
>> quite similar to the idea mentioned earlier that could be used for x86.
>>
>> Just get rid of this explicit KVM_REQ_MMU_RELOAD knowledge in
>> arch-independent code please (if you want to see this merged).
>>   
> I agree to lift the wait part to other archs later if needed, but as  
> mentioned above I could move this to arch code to the cost of one arch  
> hook more. But as also mentioned it doesn't really hurt. I agree that it  
> does not need to be KVM_REQ_MMU_RELOAD specific, we could just  
> walk/clear/wake all bits on that vcpu->requests variable.
> Would that be generic enough in your opinion ?

Don't know.

Avi?

>> Later it can all be lifted off to arch independent code.
>>   
> True for the wait part which can evolve in our arch code until it is 
> ripe to get cross arch merged.
>
> -- 
>
> Grüsse / regards, Christian Ehrhardt
> IBM Linux Technology Center, Open Virtualization