(incorrectly use in 3be98be4e9f)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
currently on ppc32 the linking fails:
CC migration/postcopy-ram.o
...
LINK microblaze-softmmu/qemu-system-microblaze
../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_end':
migration/postcopy-ram.c:717: undefined reference to `__atomic_fetch_add_8'
migration/postcopy-ram.c:738: undefined reference to `__atomic_fetch_add_8'
../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_begin':
migration/postcopy-ram.c:651: undefined reference to `__atomic_exchange_8'
migration/postcopy-ram.c:652: undefined reference to `__atomic_exchange_8'
migration/postcopy-ram.c:661: undefined reference to `__atomic_exchange_8'
collect2: error: ld returned 1 exit status
Makefile:193: recipe for target 'qemu-system-microblaze' failed
make[1]: *** [qemu-system-microblaze] Error 1
with this patch the compilation fails:
CC migration/postcopy-ram.o
In file included from include/qemu/osdep.h:36:0,
from migration/postcopy-ram.c:19:
migration/postcopy-ram.c: In function 'mark_postcopy_blocktime_begin':
include/qemu/compiler.h:86:30: error: static assertion failed: "not expecting: sizeof(*&dc->last_begin) > ATOMIC_REG_SIZE"
#define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x)
^
include/qemu/atomic.h:183:5: note: in expansion of macro 'QEMU_BUILD_BUG_ON'
QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
^
migration/postcopy-ram.c:651:5: note: in expansion of macro 'atomic_xchg'
atomic_xchg(&dc->last_begin, now_ms);
^
migration/postcopy-ram.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 7814da5b4b..6ecc1aa820 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -648,17 +648,17 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
atomic_inc(&dc->smp_cpus_down);
}
- atomic_xchg__nocheck(&dc->last_begin, now_ms);
- atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms);
- atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr);
+ atomic_xchg(&dc->last_begin, now_ms);
+ atomic_xchg(&dc->page_fault_vcpu_time[cpu], now_ms);
+ atomic_xchg(&dc->vcpu_addr[cpu], addr);
/* check it here, not at the begining of the function,
* due to, check could accur early than bitmap_set in
* qemu_ufd_copy_ioctl */
already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
if (already_received) {
- atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0);
- atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0);
+ atomic_xchg(&dc->vcpu_addr[cpu], 0);
+ atomic_xchg(&dc->page_fault_vcpu_time[cpu], 0);
atomic_dec(&dc->smp_cpus_down);
}
trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu],
@@ -719,7 +719,7 @@ static void mark_postcopy_blocktime_end(uintptr_t addr)
read_vcpu_time == 0) {
continue;
}
- atomic_xchg__nocheck(&dc->vcpu_addr[i], 0);
+ atomic_xchg(&dc->vcpu_addr[i], 0);
vcpu_blocktime = now_ms - read_vcpu_time;
affected_cpu += 1;
/* we need to know is that mark_postcopy_end was due to
--
2.15.1
On Fri, Jan 19, 2018 at 11:39 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > (incorrectly use in 3be98be4e9f) > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > currently on ppc32 the linking fails: Well armel and armhf hosts are also failing since 3be98be4e9f. > > CC migration/postcopy-ram.o > ... > LINK microblaze-softmmu/qemu-system-microblaze > ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_end': > migration/postcopy-ram.c:717: undefined reference to `__atomic_fetch_add_8' > migration/postcopy-ram.c:738: undefined reference to `__atomic_fetch_add_8' > ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_begin': > migration/postcopy-ram.c:651: undefined reference to `__atomic_exchange_8' > migration/postcopy-ram.c:652: undefined reference to `__atomic_exchange_8' > migration/postcopy-ram.c:661: undefined reference to `__atomic_exchange_8' > collect2: error: ld returned 1 exit status > Makefile:193: recipe for target 'qemu-system-microblaze' failed > make[1]: *** [qemu-system-microblaze] Error 1 > > with this patch the compilation fails: > > CC migration/postcopy-ram.o > In file included from include/qemu/osdep.h:36:0, > from migration/postcopy-ram.c:19: > migration/postcopy-ram.c: In function 'mark_postcopy_blocktime_begin': > include/qemu/compiler.h:86:30: error: static assertion failed: "not expecting: sizeof(*&dc->last_begin) > ATOMIC_REG_SIZE" > #define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x) > ^ > include/qemu/atomic.h:183:5: note: in expansion of macro 'QEMU_BUILD_BUG_ON' > QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \ > ^ > migration/postcopy-ram.c:651:5: note: in expansion of macro 'atomic_xchg' > atomic_xchg(&dc->last_begin, now_ms); > ^ > > migration/postcopy-ram.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index 7814da5b4b..6ecc1aa820 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -648,17 +648,17 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, > atomic_inc(&dc->smp_cpus_down); > } > > - atomic_xchg__nocheck(&dc->last_begin, now_ms); > - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms); > - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr); > + atomic_xchg(&dc->last_begin, now_ms); > + atomic_xchg(&dc->page_fault_vcpu_time[cpu], now_ms); > + atomic_xchg(&dc->vcpu_addr[cpu], addr); > > /* check it here, not at the begining of the function, > * due to, check could accur early than bitmap_set in > * qemu_ufd_copy_ioctl */ > already_received = ramblock_recv_bitmap_test(rb, (void *)addr); > if (already_received) { > - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0); > - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0); > + atomic_xchg(&dc->vcpu_addr[cpu], 0); > + atomic_xchg(&dc->page_fault_vcpu_time[cpu], 0); > atomic_dec(&dc->smp_cpus_down); > } > trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu], > @@ -719,7 +719,7 @@ static void mark_postcopy_blocktime_end(uintptr_t addr) > read_vcpu_time == 0) { > continue; > } > - atomic_xchg__nocheck(&dc->vcpu_addr[i], 0); > + atomic_xchg(&dc->vcpu_addr[i], 0); > vcpu_blocktime = now_ms - read_vcpu_time; > affected_cpu += 1; > /* we need to know is that mark_postcopy_end was due to > -- > 2.15.1 >
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote: > (incorrectly use in 3be98be4e9f) > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> I'm a bit confused; isnt the only difference between the nocheck versions that it'll fail at compile time instead of link? Dave > --- > currently on ppc32 the linking fails: > > CC migration/postcopy-ram.o > ... > LINK microblaze-softmmu/qemu-system-microblaze > ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_end': > migration/postcopy-ram.c:717: undefined reference to `__atomic_fetch_add_8' > migration/postcopy-ram.c:738: undefined reference to `__atomic_fetch_add_8' > ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_begin': > migration/postcopy-ram.c:651: undefined reference to `__atomic_exchange_8' > migration/postcopy-ram.c:652: undefined reference to `__atomic_exchange_8' > migration/postcopy-ram.c:661: undefined reference to `__atomic_exchange_8' > collect2: error: ld returned 1 exit status > Makefile:193: recipe for target 'qemu-system-microblaze' failed > make[1]: *** [qemu-system-microblaze] Error 1 > > with this patch the compilation fails: > > CC migration/postcopy-ram.o > In file included from include/qemu/osdep.h:36:0, > from migration/postcopy-ram.c:19: > migration/postcopy-ram.c: In function 'mark_postcopy_blocktime_begin': > include/qemu/compiler.h:86:30: error: static assertion failed: "not expecting: sizeof(*&dc->last_begin) > ATOMIC_REG_SIZE" > #define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x) > ^ > include/qemu/atomic.h:183:5: note: in expansion of macro 'QEMU_BUILD_BUG_ON' > QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \ > ^ > migration/postcopy-ram.c:651:5: note: in expansion of macro 'atomic_xchg' > atomic_xchg(&dc->last_begin, now_ms); > ^ > > migration/postcopy-ram.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index 7814da5b4b..6ecc1aa820 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -648,17 +648,17 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, > atomic_inc(&dc->smp_cpus_down); > } > > - atomic_xchg__nocheck(&dc->last_begin, now_ms); > - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms); > - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr); > + atomic_xchg(&dc->last_begin, now_ms); > + atomic_xchg(&dc->page_fault_vcpu_time[cpu], now_ms); > + atomic_xchg(&dc->vcpu_addr[cpu], addr); > > /* check it here, not at the begining of the function, > * due to, check could accur early than bitmap_set in > * qemu_ufd_copy_ioctl */ > already_received = ramblock_recv_bitmap_test(rb, (void *)addr); > if (already_received) { > - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0); > - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0); > + atomic_xchg(&dc->vcpu_addr[cpu], 0); > + atomic_xchg(&dc->page_fault_vcpu_time[cpu], 0); > atomic_dec(&dc->smp_cpus_down); > } > trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu], > @@ -719,7 +719,7 @@ static void mark_postcopy_blocktime_end(uintptr_t addr) > read_vcpu_time == 0) { > continue; > } > - atomic_xchg__nocheck(&dc->vcpu_addr[i], 0); > + atomic_xchg(&dc->vcpu_addr[i], 0); > vcpu_blocktime = now_ms - read_vcpu_time; > affected_cpu += 1; > /* we need to know is that mark_postcopy_end was due to > -- > 2.15.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 01/19/2018 03:01 PM, Dr. David Alan Gilbert wrote: > * Philippe Mathieu-Daudé (f4bug@amsat.org) wrote: >> (incorrectly use in 3be98be4e9f) >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > I'm a bit confused; isnt the only difference between the nocheck > versions that it'll fail at compile time instead of link? You are right, this isn't the right approach. Peter gave a clearer explanation here: https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg02103.html ''' because atomic operations on types larger than the host pointer size are not portable to all architectures. This should probably use the atomic_cmpxchg(), not __nocheck, because the latter bypasses the build time assert for this problem and turns a "fails on any 32-bit host" into "fails obscurely on some architectures only". ''' Maybe we should remove the __nocheck() functions and inline them in the 'checked' functions? There is no performance cost doing this, right? > > Dave > >> --- >> currently on ppc32 the linking fails: >> >> CC migration/postcopy-ram.o >> ... >> LINK microblaze-softmmu/qemu-system-microblaze >> ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_end': >> migration/postcopy-ram.c:717: undefined reference to `__atomic_fetch_add_8' >> migration/postcopy-ram.c:738: undefined reference to `__atomic_fetch_add_8' >> ../migration/postcopy-ram.o: In function `mark_postcopy_blocktime_begin': >> migration/postcopy-ram.c:651: undefined reference to `__atomic_exchange_8' >> migration/postcopy-ram.c:652: undefined reference to `__atomic_exchange_8' >> migration/postcopy-ram.c:661: undefined reference to `__atomic_exchange_8' >> collect2: error: ld returned 1 exit status >> Makefile:193: recipe for target 'qemu-system-microblaze' failed >> make[1]: *** [qemu-system-microblaze] Error 1 >> >> with this patch the compilation fails: >> >> CC migration/postcopy-ram.o >> In file included from include/qemu/osdep.h:36:0, >> from migration/postcopy-ram.c:19: >> migration/postcopy-ram.c: In function 'mark_postcopy_blocktime_begin': >> include/qemu/compiler.h:86:30: error: static assertion failed: "not expecting: sizeof(*&dc->last_begin) > ATOMIC_REG_SIZE" >> #define QEMU_BUILD_BUG_ON(x) _Static_assert(!(x), "not expecting: " #x) >> ^ >> include/qemu/atomic.h:183:5: note: in expansion of macro 'QEMU_BUILD_BUG_ON' >> QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \ >> ^ >> migration/postcopy-ram.c:651:5: note: in expansion of macro 'atomic_xchg' >> atomic_xchg(&dc->last_begin, now_ms); >> ^ >> >> migration/postcopy-ram.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c >> index 7814da5b4b..6ecc1aa820 100644 >> --- a/migration/postcopy-ram.c >> +++ b/migration/postcopy-ram.c >> @@ -648,17 +648,17 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, >> atomic_inc(&dc->smp_cpus_down); >> } >> >> - atomic_xchg__nocheck(&dc->last_begin, now_ms); >> - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], now_ms); >> - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], addr); >> + atomic_xchg(&dc->last_begin, now_ms); >> + atomic_xchg(&dc->page_fault_vcpu_time[cpu], now_ms); >> + atomic_xchg(&dc->vcpu_addr[cpu], addr); >> >> /* check it here, not at the begining of the function, >> * due to, check could accur early than bitmap_set in >> * qemu_ufd_copy_ioctl */ >> already_received = ramblock_recv_bitmap_test(rb, (void *)addr); >> if (already_received) { >> - atomic_xchg__nocheck(&dc->vcpu_addr[cpu], 0); >> - atomic_xchg__nocheck(&dc->page_fault_vcpu_time[cpu], 0); >> + atomic_xchg(&dc->vcpu_addr[cpu], 0); >> + atomic_xchg(&dc->page_fault_vcpu_time[cpu], 0); >> atomic_dec(&dc->smp_cpus_down); >> } >> trace_mark_postcopy_blocktime_begin(addr, dc, dc->page_fault_vcpu_time[cpu], >> @@ -719,7 +719,7 @@ static void mark_postcopy_blocktime_end(uintptr_t addr) >> read_vcpu_time == 0) { >> continue; >> } >> - atomic_xchg__nocheck(&dc->vcpu_addr[i], 0); >> + atomic_xchg(&dc->vcpu_addr[i], 0); >> vcpu_blocktime = now_ms - read_vcpu_time; >> affected_cpu += 1; >> /* we need to know is that mark_postcopy_end was due to >> -- >> 2.15.1 >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On 01/19/2018 10:20 AM, Philippe Mathieu-Daudé wrote: > Maybe we should remove the __nocheck() functions and inline them in the > 'checked' functions? No, I use them in TCG, properly protected with CONFIG_ATOMIC*. r~
© 2016 - 2024 Red Hat, Inc.