Re: [PATCH 1/6] kvm-s390: Fix memory slot versus run

[Avi Kivity <avi@redhat.com>]

Christian Ehrhardt wrote:
>>> I thought about implementing it with slots_lock, vcpu->request, etc 
>>> but it really looks like overkill for s390.
>>
>> We could make (some of) it common code, so it won't look so bad.  
>> There's value in having all kvm ports do things similarly; though of 
>> course we shouldn't force the solution when it isn't really needed.
>>
>> vcpu->requests is useful whenever we modify global VM state that 
>> needs to be seen by all vcpus in host mode; see  
>> kvm_reload_remote_mmus().
> yeah I read that code after your first hint in that thread, and I 
> agree that merging some of this into common code might be good.
> But in my opinion not now for this bugfix patch (the intention is just 
> to prevent a user being able to crash the host via vcpu create,set 
> mem& and vcpu run in that order).
> It might be a good point to further streamline this once we use the 
> same userspace code, but I think it doesn't make sense yet.

Sure, don't mix bugfixes with infrastructure changes, when possible.

>>> At least today we can assume that we only have one memslot. 
>>> Therefore a set_memslot with already created vcpu's will still not 
>>> interfere with running vcpus (they can't run without memslot and 
>>> since we have only one they won't run).
>>> Anyway I the code is prepared to "meet" running vcpus, because it 
>>> might be different in future. To prevent the livelock issue I 
>>> changed the code using mutex_trylock and in case I can't get the 
>>> lock I explicitly let the vcpu exit from guest.
>>
>> Why not do it unconditionally?
>>
> hmm I might have written that misleading - eventually it's a loop 
> until it got the lock
>  while !trylock
>    kick vcpu out of guest
>    schedule
>
> There is no reason to kick out guests where I got the lock cleanly as 
> far as I see.
> Especially as I expect the vcpus not running in the common case as i 
> explained above (can't run without memslot + we only have one => no 
> vcpu will run).

Still livelockable, unless you stop the vcpu from entering the guest 
immediately.

That's why vcpu->requests is so powerful.  Not only you kick the vcpu 
out of guest mode, you force it to synchronize when it tries to enter again.

-- 
error compiling committee.c: too many arguments to function