From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35142) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDaJt-00008V-F9 for qemu-devel@nongnu.org; Fri, 10 Jul 2015 11:34:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZDaJq-0000Kk-21 for qemu-devel@nongnu.org; Fri, 10 Jul 2015 11:34:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDaJp-0000Ke-TO for qemu-devel@nongnu.org; Fri, 10 Jul 2015 11:34:29 -0400 References: <1436541553-26576-1-git-send-email-fred.konrad@greensocs.com> <1436541553-26576-2-git-send-email-fred.konrad@greensocs.com> <559FE32C.5030609@redhat.com> <559FE58C.6040400@greensocs.com> From: Paolo Bonzini Message-ID: <559FE600.7040500@redhat.com> Date: Fri, 10 Jul 2015 17:34:24 +0200 MIME-Version: 1.0 In-Reply-To: <559FE58C.6040400@greensocs.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 1/3] cpus: protect queued_work_* with work_mutex. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Frederic Konrad , qemu-devel@nongnu.org, mttcg@greensocs.com Cc: mark.burton@greensocs.com, alex.bennee@linaro.org, a.rigo@virtualopensystems.com, guillaume.delbergue@greensocs.com On 10/07/2015 17:32, Frederic Konrad wrote: >>> > > I think something like that can work because we don't have two > flush_queued_work at the same time on the same CPU? Yes, this works; there is only one consumer. Holding locks within a callback can be very painful, especially if there is a chance that the callback will take a very coarse lock such as big QEMU lock. It can cause AB-BA deadlocks. Paolo > static void flush_queued_work(CPUState *cpu) > { > struct qemu_work_item *wi; > > if (cpu->queued_work_first == NULL) { > return; > } > > qemu_mutex_lock(&cpu->work_mutex); > while ((wi = cpu->queued_work_first)) { > cpu->queued_work_first = wi->next; > qemu_mutex_unlock(&cpu->work_mutex); > wi->func(wi->data); > qemu_mutex_lock(&cpu->work_mutex); > wi->done = true; > if (wi->free) { > g_free(wi); > } > } > cpu->queued_work_last = NULL; > qemu_mutex_unlock(&cpu->work_mutex); > > qemu_cond_broadcast(&qemu_work_cond); > }