From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brQOb-0007Fo-0R for qemu-devel@nongnu.org; Tue, 04 Oct 2016 10:08:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brQOV-0008Uo-Mh for qemu-devel@nongnu.org; Tue, 04 Oct 2016 10:08:36 -0400 Received: from mail-lf0-x22b.google.com ([2a00:1450:4010:c07::22b]:35765) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brQOV-0008Tu-95 for qemu-devel@nongnu.org; Tue, 04 Oct 2016 10:08:31 -0400 Received: by mail-lf0-x22b.google.com with SMTP id l131so184892504lfl.2 for ; Tue, 04 Oct 2016 07:08:31 -0700 (PDT) References: <20160930213106.20186-1-alex.bennee@linaro.org> <20160930213106.20186-4-alex.bennee@linaro.org> <3cb98ad6-1832-57af-e06d-450a031c15f1@redhat.com> <87h98tu69k.fsf@linaro.org> <9cc2a403-2116-2c8b-9573-62f1fd316fcf@redhat.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <9cc2a403-2116-2c8b-9573-62f1fd316fcf@redhat.com> Date: Tue, 04 Oct 2016 15:08:28 +0100 Message-ID: <87oa30ryub.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 03/15] exec-all.h: revert tb_page_addr_t to target_ulong List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, mttcg@greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, bobby.prani@gmail.com, nikunj@linux.vnet.ibm.com, mark.burton@greensocs.com, jan.kiszka@siemens.com, serge.fdrv@gmail.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite Paolo Bonzini writes: > On 03/10/2016 11:32, Alex Bennée wrote: >> 1. For atomic_read/set fall back to plain access for sizeof(*ptr) > >> sizeof(void *) >> >> This would throw up warnings in ThreadSanitizer on 64-on-32 but >> practically generate the same code as before. >> >> 2. Split sizeof(*ptr) > sizeof(void *) accesses over two >> >> This would keep the sanitizer happy but be incorrect behaviour, you >> could get tears. >> >> 3. Add a generic 64-on-32 lock for these accesses >> >> It would be a global lock for any oversized access which could end up >> being highly contended but at least any behaviour would be always >> correct. > > (3) is what libatomic does. > > For generic statistics/counters I have written, but not yet upstreamed, > a module which defines two abstract data types: > > - Stat64 is a 64-bit value and it can do 64-bit add/min/max. Reads > block writes, but writes try to bypass the lock if possible (e.g. > min/max that don't modify the stored value, or add that only touches the > lower 32-bits). > > - Count64 is actually a 63-bit value and can do 32-bit add or 63-bit > get/set. Writes block reads, but 32-bit adds try pretty hard not to. > > These are meant for the block layer, so they would be necessary even if > 64-on-32 went away. Unfortunately, neither is a perfect replacement for > a 64-bit variable... With rth's CONFIG_ATOMIC64 patch mingw doesn't complain with this: atomic.h: relax sizeof(*ptr) > sizeof(*void) check If the compiler can already handle oversized accesses let it do so. Signed-off-by: Alex Bennée 1 file changed, 10 insertions(+), 4 deletions(-) include/qemu/atomic.h | 14 ++++++++++---- modified include/qemu/atomic.h @@ -99,15 +99,21 @@ * no effect on the generated code but not using the atomic primitives * will get flagged by sanitizers as a violation. */ +#ifdef CONFIG_ATOMIC64 +#define ATOMIC_BUILD_BUG_ON(ptr) QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(uint64_t)); +#else +#define ATOMIC_BUILD_BUG_ON(ptr) QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); +#endif + #define atomic_read(ptr) \ ({ \ - QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - __atomic_load_n(ptr, __ATOMIC_RELAXED); \ + ATOMIC_BUILD_BUG_ON(ptr); \ + __atomic_load_n(ptr, __ATOMIC_RELAXED); \ }) #define atomic_set(ptr, i) do { \ - QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \ - __atomic_store_n(ptr, i, __ATOMIC_RELAXED); \ + ATOMIC_BUILD_BUG_ON(ptr); \ + __atomic_store_n(ptr, i, __ATOMIC_RELAXED); \ } while(0) /* See above: most compilers currently treat consume and acquire the