qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on


From: Dr. David Alan Gilbert
Subject: Re: [Qemu-devel] [PATCH V6 08/10] migration: calculate vCPU blocktime on dst side
Date: Thu, 1 Jun 2017 11:07:13 +0100
User-agent: Mutt/1.8.2 (2017-04-18)

* Peter Xu (address@hidden) wrote:

> > > Why use *__nocheck() rather than atomic_xchg() or even atomic_read()?
> > > Same thing happened a lot in current patch.
> > atomic_read/atomic_xchg for mingw/(gcc on arm32) has build time check,
> > 
> > QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));
> > 
> > it prevents using 64 atomic operation on 32 architecture, just mingw I
> > think, but postcopy-ram.c isn't compiling for mingw.
> > On other 32 platforms as I know clang/gcc allow to use 8 bytes
> > long variables in built atomic operations. In arm32 it allows in
> > builtin. But QEMU on arm32 still
> > has that sanity check, and I think it's bug, so I just worked it around.
> > Maybe better was to fix it.
> > 
> > I tested in docker, using follow command:
> > make address@hidden
> > 
> > And got following error
> > 
> > /tmp/qemu-test/src/migration/postcopy-ram.c: In function
> > 'mark_postcopy_blocktime_begin':
> > /tmp/qemu-test/src/include/qemu/compiler.h:86:30: error: static
> > assertion failed: "not expecting: sizeof(*&dc->vcpu_addr[cpu]) >
> > sizeof(void *)"
> >  #define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x)
> > 
> > when I used atomic_xchg,
> > I agree with you, but I think need to fix atomic.h firstly and add 
> > additional
> > #ifdef there.
> > 
> > And I didn't want to split 64 bit values onto 32 bit values, but I saw
> > in mailing list people are doing it.
> 
> If this is a bug, then I guess the best way is to fix it. But before
> that - can a 32bit system really do 64bit atomic ops? Is it really a
> bug at all?

Some can, but not all.
ARM has 'ldrexd' for 64bit atomics; but I can't remember which subset
of ARMs has it; I think most recent stuff.
I don't think you can guarantee it's on all architectures though, but it
might be on any we care about.

Dave

> Thanks,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK



reply via email to

[Prev in Thread] Current Thread [Next in Thread]