[sheepdog] [PATCH] qemu/atomic.h: prefix qemu_ to solve <stdatomic.h> collisions
Eric Blake
eblake at redhat.com
Mon Sep 21 23:29:10 CEST 2020
On 9/21/20 11:23 AM, Stefan Hajnoczi wrote:
> clang's C11 atomic_fetch_*() functions only take a C11 atomic type
> pointer argument. QEMU uses direct types (int, etc) and this causes a
> compiler error when a QEMU code calls these functions in a source file
> that also included <stdatomic.h> via a system header file:
>
> $ CC=clang CXX=clang++ ./configure ... && make
> ../util/async.c:79:17: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid)
>
> Avoid using atomic_*() names in QEMU's atomic.h since that namespace is
> used by <stdatomic.h>. Prefix QEMU's APIs with qemu_ so that atomic.h
> and <stdatomic.h> can co-exist.
>
> This patch was generated using:
>
> $ git diff | grep -o '\<atomic_[a-z0-9_]\+' | sort -u >/tmp/changed_identifiers
Missing a step in the recipe: namely, you probably modified
include/qemu/atomic*.h prior to running 'git diff' (so that you actually
had input to feed to grep -o). But spelling it 'git diff HEAD^
include/qemu/atomic*.h | ...' does indeed give me a sane list of
identifiers that looks like what you touched in the rest of the patch.
> $ for identifier in $(</tmp/changed_identifiers64); do \
Also not quite the right recipe, based on the file name used in the line
above.
> sed -i "s%\<$identifier\>%qemu_$identifier%" $(git grep -l "\<$identifier\>") \
> done
>
Fortunately, running "git grep -c '\<atomic_[a-z0-9_]\+'" on the
pre-patch state of the tree gives me a list that is somewhat close to
yours, where the obvious difference in line counts is explained by:
> I manually fixed line-wrap issues and misaligned rST tables.
>
> Signed-off-by: Stefan Hajnoczi <stefanha at redhat.com>
> ---
First, focusing on the change summary:
> docs/devel/lockcnt.txt | 14 +-
> docs/devel/rcu.txt | 40 +--
> accel/tcg/atomic_template.h | 20 +-
> include/block/aio-wait.h | 4 +-
> include/block/aio.h | 8 +-
> include/exec/cpu_ldst.h | 2 +-
> include/exec/exec-all.h | 6 +-
> include/exec/log.h | 6 +-
> include/exec/memory.h | 2 +-
> include/exec/ram_addr.h | 27 +-
> include/exec/ramlist.h | 2 +-
> include/exec/tb-lookup.h | 4 +-
> include/hw/core/cpu.h | 2 +-
> include/qemu/atomic.h | 258 +++++++-------
> include/qemu/atomic128.h | 6 +-
These two are the most important for the sake of this patch; perhaps
it's worth a temporary override of your git orderfile if you have to
respin, to list them first?
> include/qemu/bitops.h | 2 +-
> include/qemu/coroutine.h | 2 +-
> include/qemu/log.h | 6 +-
> include/qemu/queue.h | 8 +-
> include/qemu/rcu.h | 10 +-
> include/qemu/rcu_queue.h | 103 +++---
Presumably, this and any other file with an odd number of changes was
due to a difference in lines after reformatting long lines.
> include/qemu/seqlock.h | 8 +-
...
> util/stats64.c | 34 +-
> docs/devel/atomics.rst | 326 +++++++++---------
> .../opensbi-riscv32-generic-fw_dynamic.elf | Bin 558668 -> 558698 bytes
> .../opensbi-riscv64-generic-fw_dynamic.elf | Bin 620424 -> 620454 bytes
Why are we regenerating .elf files in this patch? Is your change even
correct for those two files?
> scripts/kernel-doc | 2 +-
> tcg/aarch64/tcg-target.c.inc | 2 +-
> tcg/mips/tcg-target.c.inc | 2 +-
> tcg/ppc/tcg-target.c.inc | 6 +-
> tcg/sparc/tcg-target.c.inc | 5 +-
> 135 files changed, 1195 insertions(+), 1130 deletions(-)
I don't spot accel/tcg/atomic_common.c.inc in the list (which declares
functions such as atomic_trace_rmw_pre) - I guess that's intentional
based on how you tried to edit only the identifiers you touched in
include/qemu/atomic*.h.
For the rest of this patch, I only spot-checked in places, trusting the
mechanical nature of this patch, and not spotting anything wrong in the
places I checked. But the two .elf files worry me enough to withhold
R-b. At the same time, because it's big, it will probably be a source
of conflicts if we don't get it in soon, but can also be regenerated (if
your recipe is corrected) without too much difficulty. So I am in favor
of the idea.
> diff --git a/pc-bios/opensbi-riscv32-generic-fw_dynamic.elf b/pc-bios/opensbi-riscv32-generic-fw_dynamic.elf
> index eb9ebf5674d3953ab25de6bf4db134abe904eb20..35b80512446dcf5c49424ae90caacf3c579be1b5 100644
> GIT binary patch
> delta 98
> zcmX at pqx7mrsiB3jg{g(Pg=Gt?!4uZP)ZEhe?LdZ at 5QIJ5?Hg+mgxS918!HgAZQt>Y
> ceSHN~X?i|K5fhYsvykI97nHrFhGPaN0Hp#a^8f$<
>
> delta 62
> zcmaFWqjaW6siB3jg{g(Pg=Gt?!ISN#Pguo-rU!guEowjhjTMO5wjck-zP at 66bv{QC
> S)Amn=9Jjf)U#j7l!3h9Zj2qGb
>
> diff --git a/pc-bios/opensbi-riscv64-generic-fw_dynamic.elf b/pc-bios/opensbi-riscv64-generic-fw_dynamic.elf
> index 642a64e240d09b2ddb9fc12c74718ae8d386b9d3..9cf2cf23b747cb5be1d3389a0611d8697609c6f8 100644
> GIT binary patch
> delta 102
> zcmeBpue$8LYC{WS3sVbo3(FSPMGsgDQ*%q>w*wh6LJ;=!eV<s1Ak21y&#XYq2E^>!
> e4L)<s&w(mGAJ19D1Z6uWaUSP_vN>`&8 at K>*RVYvZ
>
> delta 66
> zcmZ4XUbW-BYC{WS3sVbo3(FSPMGv+wf50juH2uUU)}nU%&#XYq2E^>!?LTwO&)NPs
> Up0kK)dsGtVajxxZxttAL0Np<vJ^%m!
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 030b5c8691..9ec38a1bf1 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1625,7 +1625,7 @@ sub dump_function($$) {
> # If you mess with these regexps, it's a good idea to check that
> # the following functions' documentation still comes out right:
> # - parport_register_device (function pointer parameters)
> - # - atomic_set (macro)
> + # - qemu_atomic_set (macro)
> # - pci_match_device, __copy_to_user (long return type)
Does the result of sphinx still look good, as mentioned in this comment?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
More information about the sheepdog
mailing list