From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/6] kvm-s390: Fix memory slot versus run Date: Mon, 11 May 2009 17:02:27 +0300 Message-ID: <4A082FF3.4060908@redhat.com> References: <1241534358-32172-1-git-send-email-ehrhardt@linux.vnet.ibm.com> <1241534358-32172-2-git-send-email-ehrhardt@linux.vnet.ibm.com> <4A017C2E.6060306@redhat.com> <4A08217E.6000102@linux.vnet.ibm.com> <4A0824EF.5010201@redhat.com> <4A082C4E.60501@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Christian Borntraeger , Carsten Otte To: Christian Ehrhardt Return-path: Received: from mx2.redhat.com ([66.187.237.31]:57493 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751382AbZEKOCe (ORCPT ); Mon, 11 May 2009 10:02:34 -0400 In-Reply-To: <4A082C4E.60501@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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