From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8VNa-0007n6-Ad for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:21:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8VNW-0003bz-R7 for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:21:54 -0400 Received: from mail-io0-x244.google.com ([2607:f8b0:4001:c06::244]:34082) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8VNW-0003bo-K0 for qemu-devel@nongnu.org; Thu, 02 Jun 2016 12:21:50 -0400 Received: by mail-io0-x244.google.com with SMTP id l9so7173280ioe.1 for ; Thu, 02 Jun 2016 09:21:50 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20160526163549.3276-1-a.rigo@virtualopensystems.com> <20160526163549.3276-2-a.rigo@virtualopensystems.com> From: alvise rigo Date: Thu, 2 Jun 2016 18:21:35 +0200 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [RFC 01/10] exec: Introduce tcg_exclusive_{lock, unlock}() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pranith Kumar Cc: MTTCG Devel , =?UTF-8?B?QWxleCBCZW5uw6ll?= , qemu-devel , Jani Kokkonen , Claudio Fontana , VirtualOpenSystems Technical Team , =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= , Paolo Bonzini , Richard Henderson , Sergey Fedorov , "Emilio G. Cota" , Peter Maydell Hi Pranith, Thank you for the hint, I will keep this in mind for the next version. Regards, alvise On Tue, May 31, 2016 at 5:03 PM, Pranith Kumar wrote: > Hi Alvise, > > On Thu, May 26, 2016 at 12:35 PM, Alvise Rigo > wrote: >> Add tcg_exclusive_{lock,unlock}() functions that will be used for making >> the emulation of LL and SC instructions thread safe. >> >> Signed-off-by: Alvise Rigo > > > >> +__thread bool cpu_have_exclusive_lock; >> +QemuSpin cpu_exclusive_lock; >> +inline void tcg_exclusive_lock(void) >> +{ >> + if (!cpu_have_exclusive_lock) { >> + qemu_spin_lock(&cpu_exclusive_lock); >> + cpu_have_exclusive_lock = true; >> + } >> +} >> + >> +inline void tcg_exclusive_unlock(void) >> +{ >> + if (cpu_have_exclusive_lock) { >> + cpu_have_exclusive_lock = false; >> + qemu_spin_unlock(&cpu_exclusive_lock); >> + } >> +} > > I think the unlock() here should have an assert if > cpu_have_exclusive_lock is false. From what I can see, a thread should > either take the exclusive lock or wait spinning for it in lock(). So > unlock() should always see cpu_have_exclusive_lock as true. It is a > good place to find locking bugs. > > -- > Pranith