From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayL9o-0001Wb-Mh for qemu-devel@nongnu.org; Thu, 05 May 2016 11:25:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ayL9d-0007Op-0j for qemu-devel@nongnu.org; Thu, 05 May 2016 11:25:35 -0400 Received: from mail-lf0-x234.google.com ([2a00:1450:4010:c07::234]:33228) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayL9b-0007Jb-Nh for qemu-devel@nongnu.org; Thu, 05 May 2016 11:25:28 -0400 Received: by mail-lf0-x234.google.com with SMTP id y84so100470014lfc.0 for ; Thu, 05 May 2016 08:25:13 -0700 (PDT) References: <1459870344-16773-1-git-send-email-alex.bennee@linaro.org> <1459870344-16773-5-git-send-email-alex.bennee@linaro.org> <572B5684.1030001@gmail.com> <87k2j8r1hz.fsf@linaro.org> From: Sergey Fedorov Message-ID: <572B65D0.2090203@gmail.com> Date: Thu, 5 May 2016 18:25:04 +0300 MIME-Version: 1.0 In-Reply-To: <87k2j8r1hz.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v2 04/11] tcg: comment on which functions have to be called with tb_lock held List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: mttcg@listserver.greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, qemu-devel@nongnu.org, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite , =?UTF-8?Q?Andreas_F=c3=a4rber?= On 05/05/16 18:03, Alex Bennée wrote: > Sergey Fedorov writes: > >> On 05/04/16 18:32, Alex Bennée wrote: >>> diff --git a/exec.c b/exec.c >>> index f46e596..17f390e 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -826,6 +826,7 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, >>> { >>> CPUBreakpoint *bp; >>> >>> + /* TODO: locking (RCU?) */ >>> bp = g_malloc(sizeof(*bp)); >>> >>> bp->pc = pc; >> This comment is a little inconsistent. We should make access to >> breakpoint and watchpoint lists to be thread-safe in all the functions >> using them. So if we note this, it should be noted in all such places. >> Also, it's probably not a good idea to put such comment just above >> g_malloc() invocation, it could be a bit confusing. A bit more details >> would also be nice. > Good point. > > I could really do with some tests to exercise the debugging interface. I > did some when I wrote the arm kvm GDB stuff (see > 261f4d6d3e5445f887e070f047968e756c30cf06) but it is a) not plumbed in > and b) not really a stress test which is what you want to be sure your > handling is thread safe. It seems we can only have a race between TCG helper inserting/removing break-/watchpoint and gdbstub. So maybe we could just use separate lists for CPU and GDB breakpoints? Kind regards, Sergey