From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41084) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZczr-0007Z9-9S for qemu-devel@nongnu.org; Wed, 09 Sep 2015 06:53:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZczo-0000Em-K0 for qemu-devel@nongnu.org; Wed, 09 Sep 2015 06:52:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44773) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZczo-0000Ec-FH for qemu-devel@nongnu.org; Wed, 09 Sep 2015 06:52:56 -0400 References: <1441732357-11861-1-git-send-email-jjherne@linux.vnet.ibm.com> <1441732357-11861-2-git-send-email-jjherne@linux.vnet.ibm.com> <87egi77u3f.fsf@neno.neno> From: Paolo Bonzini Message-ID: <55F00F83.2080708@redhat.com> Date: Wed, 9 Sep 2015 12:52:51 +0200 MIME-Version: 1.0 In-Reply-To: <87egi77u3f.fsf@neno.neno> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 1/5] cpu: Provide vcpu throttling interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com, "Jason J. Herne" Cc: amit.shah@redhat.com, borntraeger@de.ibm.com, qemu-devel@nongnu.org, afaerber@suse.de, dgilbert@redhat.com On 09/09/2015 12:41, Juan Quintela wrote: >> > + qemu_mutex_unlock_iothread(); >> > + atomic_set(&cpu->throttle_thread_scheduled, 0); >> > + g_usleep(sleeptime_ns / 1000); /* Convert ns to us for usleep call */ >> > + qemu_mutex_lock_iothread(); > > Why is this thread safe? > > qemu_mutex_lock_iothread() is protecting (at least) cpu_work_first on > each cpu. How can we be sure that _nothing_ will change that while we > are waiting? You only have to be sure that the queued work list remains consistent; not that nothing changes. (BTW, there is a queued patch that moves the queued work list to its own mutex, and indeed it releases that mutex while calling the work function). > A fast look through the tree don't show anything that > runs here that drops the lock. Actually, the existing implementation of throttling does. :) Paolo