From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: Role of qemu_fair_mutex Date: Tue, 04 Jan 2011 17:12:11 +0200 Message-ID: <4D2338CB.7020900@redhat.com> References: <4D219AF5.2030204@web.de> <4D219E6D.8060902@redhat.com> <4D232BF6.6050102@codemonkey.ws> <4D232E4E.5030600@redhat.com> <4D2334D4.2020104@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Jan Kiszka , qemu-devel , kvm , Marcelo Tosatti To: Anthony Liguori Return-path: Received: from mx1.redhat.com ([209.132.183.28]:19067 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752588Ab1ADPMS (ORCPT ); Tue, 4 Jan 2011 10:12:18 -0500 In-Reply-To: <4D2334D4.2020104@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: On 01/04/2011 04:55 PM, Anthony Liguori wrote: > >>> >>> When the TCG thread, it needs to let the IO thread run for at least >>> one iteration. Coordinating the execution of the IO thread such >>> that it's guaranteed to run at least once and then having it drop >>> the qemu mutex long enough for the TCG thread to acquire it is the >>> purpose of the qemu_fair_mutex. >> >> That doesn't compute - the iothread doesn't hog the global lock (it >> sleeps most of the time, and drops the lock while sleeping), so the >> iothread cannot starve out tcg. > > The fact that the iothread drops the global lock during sleep is a > detail that shouldn't affect correctness. The IO thread is absolutely > allowed to run for arbitrary periods of time without dropping the qemu > mutex. No, it's not, since it will stop vcpus in their tracks. Whenever we hold qemu_mutex for unbounded time, that's a bug. I think the only place is live migration and perhaps tcg? > >> On the other hand, tcg does hog the global lock, so it needs to be >> made to give it up so the iothread can run, for example my completion >> example. > > It's very easy to ask TCG to give up the qemu_mutex by using > cpu_interrupt(). It will drop the qemu_mutex and it will not attempt > to acquire it again until you restart the VCPU. Maybe that's the solution: def acquire_global_mutex(): if not tcg_thread: cpu_interrupt() global_mutex.aquire() release_global_mutex(): global_mutex.release() if not tcg_thread: cpu_resume() though it's racy, two non-tcg threads can cause an early resume. > >> I think the abstraction we need here is a priority lock, with higher >> priority given to the iothread. A lock() operation that takes >> precedence would atomically signal the current owner to drop the lock. > > The I/O thread can reliably acquire the lock whenever it needs to. > > If you drop all of the qemu_fair_mutex stuff and leave the qemu_mutex > getting dropped around select, TCG will generally work reliably. But > this is not race free. What would be the impact of a race here? > Just dropping a lock does not result in reliable hand off. Why do we want a handoff in the first place? I don't think we do. I think we want the iothread to run in preference to tcg, since tcg is a lock hog under guest control, while the iothread is not a lock hog (excepting live migration). > > I think a generational counter could work and a condition could work. -- error compiling committee.c: too many arguments to function