From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD Date: Mon, 31 Jan 2011 13:18:12 +0100 Message-ID: <4D46A884.5060906@siemens.com> References: <4D417F1F.7020302@siemens.com> <4D418230.1010801@siemens.com> <4D4688EB.30408@redhat.com> <4D469C87.3080909@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , Stefan Hajnoczi To: Stefan Hajnoczi Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20598 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752452Ab1AaMSb (ORCPT ); Mon, 31 Jan 2011 07:18:31 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 2011-01-31 13:13, Stefan Hajnoczi wrote: > On Mon, Jan 31, 2011 at 11:27 AM, Jan Kiszka wrote: >> On 2011-01-31 11:03, Avi Kivity wrote: >>> On 01/27/2011 04:33 PM, Jan Kiszka wrote: >>>> Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between >>>> checking for exit_request on vcpu entry and timer signals arriving >>>> before KVM starts to catch them. Plug it by blocking both timer related >>>> signals also on !CONFIG_IOTHREAD and process those via signalfd. >>>> >>>> As this fix depends on real signalfd support (otherwise the timer >>>> signals only kick the compat helper thread, and the main thread hangs), >>>> we need to detect the invalid constellation and abort configure. >>>> >>>> Signed-off-by: Jan Kiszka >>>> CC: Stefan Hajnoczi >>>> --- >>>> >>>> I don't want to invest that much into !IOTHREAD anymore, so let's see if >>>> the proposed catch&abort is acceptable. >>>> >>> >>> I don't understand the dependency on signalfd. The normal way of doing >>> things, either waiting for the signal in sigtimedwait() or in >>> ioctl(KVM_RUN), works with SIGALRM just fine. >> >> And how would you be kicked out of the select() call if it is waiting >> with a timeout? We only have a single thread here. >> >> The only alternative is Stefan's original proposal. But that required >> fiddling with the signal mask twice per KVM_RUN. > > I think my original patch messed with the sigmask in the wrong place, > as you mentioned doing it twice per KVM_RUN isn't a good idea. I > wonder if we can enable SIGALRM only in blocking calls and guest code > execution but without signalfd. It might be possible, I don't see an > immediate problem with doing that, we might have to use pselect(2) or > similar in a few places. My main concern about alternative approaches is that IOTHREAD is about to become the default, and hardly anyone (of the few upstream KVM users) will run without it in the foreseeable future. The next step will be the removal of any !CONFIG_IOTHREAD section. So, how much do we want to invest here (provided my proposal has not remaining issues)? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48278 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pjsi4-0004CU-K8 for qemu-devel@nongnu.org; Mon, 31 Jan 2011 07:18:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pjsi3-0004vx-Az for qemu-devel@nongnu.org; Mon, 31 Jan 2011 07:18:20 -0500 Received: from goliath.siemens.de ([192.35.17.28]:20523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pjsi3-0004vr-1C for qemu-devel@nongnu.org; Mon, 31 Jan 2011 07:18:19 -0500 Message-ID: <4D46A884.5060906@siemens.com> Date: Mon, 31 Jan 2011 13:18:12 +0100 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD References: <4D417F1F.7020302@siemens.com> <4D418230.1010801@siemens.com> <4D4688EB.30408@redhat.com> <4D469C87.3080909@siemens.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Marcelo Tosatti , Stefan Hajnoczi , Avi Kivity , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" On 2011-01-31 13:13, Stefan Hajnoczi wrote: > On Mon, Jan 31, 2011 at 11:27 AM, Jan Kiszka wrote: >> On 2011-01-31 11:03, Avi Kivity wrote: >>> On 01/27/2011 04:33 PM, Jan Kiszka wrote: >>>> Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between >>>> checking for exit_request on vcpu entry and timer signals arriving >>>> before KVM starts to catch them. Plug it by blocking both timer related >>>> signals also on !CONFIG_IOTHREAD and process those via signalfd. >>>> >>>> As this fix depends on real signalfd support (otherwise the timer >>>> signals only kick the compat helper thread, and the main thread hangs), >>>> we need to detect the invalid constellation and abort configure. >>>> >>>> Signed-off-by: Jan Kiszka >>>> CC: Stefan Hajnoczi >>>> --- >>>> >>>> I don't want to invest that much into !IOTHREAD anymore, so let's see if >>>> the proposed catch&abort is acceptable. >>>> >>> >>> I don't understand the dependency on signalfd. The normal way of doing >>> things, either waiting for the signal in sigtimedwait() or in >>> ioctl(KVM_RUN), works with SIGALRM just fine. >> >> And how would you be kicked out of the select() call if it is waiting >> with a timeout? We only have a single thread here. >> >> The only alternative is Stefan's original proposal. But that required >> fiddling with the signal mask twice per KVM_RUN. > > I think my original patch messed with the sigmask in the wrong place, > as you mentioned doing it twice per KVM_RUN isn't a good idea. I > wonder if we can enable SIGALRM only in blocking calls and guest code > execution but without signalfd. It might be possible, I don't see an > immediate problem with doing that, we might have to use pselect(2) or > similar in a few places. My main concern about alternative approaches is that IOTHREAD is about to become the default, and hardly anyone (of the few upstream KVM users) will run without it in the foreseeable future. The next step will be the removal of any !CONFIG_IOTHREAD section. So, how much do we want to invest here (provided my proposal has not remaining issues)? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux