From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50309) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2bZv-0005gA-A8 for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:41:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2bZs-0003aq-2g for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:41:43 -0400 Received: from greensocs.com ([193.104.36.180]:57286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2bZr-0003af-O1 for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:41:40 -0400 Message-ID: <5577F83F.8040905@greensocs.com> Date: Wed, 10 Jun 2015 10:41:35 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1433514678-4464-1-git-send-email-fred.konrad@greensocs.com> <871thl8ctj.fsf@linaro.org> <87pp556l5d.fsf@linaro.org> <5577EF40.2010207@greensocs.com> In-Reply-To: <5577EF40.2010207@greensocs.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH] Use atomic cmpxchg to atomically check the exclusive value in a STREX Reply-To: Frederic Konrad List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: mttcg@listserver.greensocs.com, peter.maydell@linaro.org, mark.burton@greensocs.com, qemu-devel@nongnu.org, agraf@suse.de, guillaume.delbergue@greensocs.com, pbonzini@redhat.com On 10/06/2015 10:03, Frederic Konrad wrote: > On 09/06/2015 15:55, Alex Benn=C3=A9e wrote: >> Alex Benn=C3=A9e writes: >> >>> fred.konrad@greensocs.com writes: >>> >>>> From: KONRAD Frederic >>>> >>>> This mechanism replaces the existing load/store exclusive mechanism=20 >>>> which seems >>>> to be broken for multithread. >>>> It follows the intention of the existing mechanism and stores the=20 >>>> target address >>>> and data values during a load operation and checks that they remain=20 >>>> unchanged >>>> before a store. >>>> >>>> In common with the older approach, this provides weaker semantics=20 >>>> than required >>>> in that it could be that a different processor writes the same=20 >>>> value as a >>>> non-exclusive write, however in practise this seems to be irrelevant= . >> >>>> +/* Protect cpu_exclusive_* variable .*/ >>>> +__thread bool cpu_have_exclusive_lock; >>>> +QemuMutex cpu_exclusive_lock; >>>> + >>>> +inline void arm_exclusive_lock(void) >>>> +{ >>>> + if (!cpu_have_exclusive_lock) { >>>> + qemu_mutex_lock(&cpu_exclusive_lock); >>>> + cpu_have_exclusive_lock =3D true; >>>> + } >>>> +} >>>> + >>>> +inline void arm_exclusive_unlock(void) >>>> +{ >>>> + if (cpu_have_exclusive_lock) { >>>> + cpu_have_exclusive_lock =3D false; >>>> + qemu_mutex_unlock(&cpu_exclusive_lock); >>>> + } >>>> +} >>> I don't quite follow. If these locks are mean to be protecting=20 >>> access to >>> variables then how do they do that? The lock won't block if another >>> thread is currently messing with the protected values. >> Having re-read after coffee I'm still wondering why we need the >> per-thread bool? All the lock/unlock pairs are for critical sections s= o >> don't we just want to serialise on the qemu_mutex_lock(), what do the >> flags add apart from allowing you to next locks that shouldn't happen? >> >> > You are probably right, this might be a rest of the old approach. > There were branches so we needed to allow next locks. > Hmm actually it seems necessary as we mutex the entire atomic_cmpxchg64. If tlb_fill trigger an exception we re-lock the mutex. > Thanks, > Fred