From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35074) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdnrG-0005ck-W0 for qemu-devel@nongnu.org; Sat, 27 Aug 2016 20:21:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bdnrA-0002De-VS for qemu-devel@nongnu.org; Sat, 27 Aug 2016 20:21:53 -0400 Received: from mail-qk0-x22a.google.com ([2607:f8b0:400d:c09::22a]:33227) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bdnrA-0002DT-QM for qemu-devel@nongnu.org; Sat, 27 Aug 2016 20:21:48 -0400 Received: by mail-qk0-x22a.google.com with SMTP id z190so109174132qkc.0 for ; Sat, 27 Aug 2016 17:21:48 -0700 (PDT) Sender: Paolo Bonzini References: <1470158864-17651-1-git-send-email-alex.bennee@linaro.org> <1470158864-17651-12-git-send-email-alex.bennee@linaro.org> <20160802192242.GB18402@flamenco> From: Paolo Bonzini Message-ID: <1d086cf1-5492-8645-6b19-aeaf525f6e48@redhat.com> Date: Sun, 28 Aug 2016 02:21:31 +0200 MIME-Version: 1.0 In-Reply-To: <20160802192242.GB18402@flamenco> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 11/13] cpu-exec-common: Introduce async_safe_run_on_cpu() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Emilio G. Cota" , =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: mttcg@listserver.greensocs.com, peter.maydell@linaro.org, claudio.fontana@huawei.com, Sergey Fedorov , Peter Crosthwaite , jan.kiszka@siemens.com, Riku Voipio , mark.burton@greensocs.com, a.rigo@virtualopensystems.com, qemu-devel@nongnu.org, serge.fdrv@gmail.com, bobby.prani@gmail.com, rth@twiddle.net, fred.konrad@greensocs.com On 02/08/2016 21:22, Emilio G. Cota wrote: > An alternative is to have a separate lock for safe work, and check for > safe work once there are no other locks held; a good place to do this is > at the beginning of cpu_loop_exec. This scales better, and I'd argue > it's simpler. In fact, I posted a patch that does this about a year > ago (!): > https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02576.html I like the idea. > Paolo didn't like condvars, but now I see them coming up again. I guess > he still won't like the synchronize_rcu() call in there, and I don't like > it either, but I don't think that's an essential part of that patch. The problem with CPUs coming up late is indeed present in this patch, I'll review your patch on the flight. :) synchronize_rcu() is actually relatively cheap with URCU, so I guess that's fine. An alternative to that could be a pthread_barrier_t, but it can be added later. Another way to fix the issue with a variable number of waiters could be to wrap safe work with rcu_read_lock and rcu_read_unlock, and put a synchronize_rcu() at the beginning of the CPU thread function. But it can be done later too. Your patch from a year ago, right now, seems to be the best to me. I'd like to make it use regular work items instead of the special cpu->tcg_work_func, but that's pretty much it. Paolo