Re: [PATCH v4 0/9] add xenalyze to staging

[George Dunlap <george.dunlap@eu.citrix.com>]

On 06/03/2015 11:10 AM, Ian Campbell wrote:
> On Sat, 2015-05-23 at 08:24 +0000, Olaf Hering wrote:
>> Having xenalyze in the source tree makes it much easier to keep private
>> debug code in hypervisor and xenalyze in sync. It helped alot while
>> debugging the root cause for commit 607e8494c42397fb249191904066cace6ac9a880.
> 
> I'm afraid it doesn't build on arm64.
> 
> Some of these actually look like non-arch specific failures (e.g.
> conflicts with register_t from system headers) or issues which should
> probably be addressed with xenalyze in tree (e.g. NR_CPUS ought to be
> available directly now?) or with some trivial #ifdef modifications.
> 
> That said, I don't know that xentrace actually works on ARM nor that
> xenalyze could analyse such traces even with the build issues addressed,
> so I'd be equally happy if this was just made x86 only.
> 
> In file included from /usr/aarch64-linux-gnu/include/linux/types.h:4:0,
>                  from /usr/aarch64-linux-gnu/include/asm/ptrace.h:22,
>                  from /usr/aarch64-linux-gnu/include/sys/user.h:25,
>                  from /usr/aarch64-linux-gnu/include/sys/procfs.h:34,
>                  from /usr/aarch64-linux-gnu/include/sys/ucontext.h:26,
>                  from /usr/aarch64-linux-gnu/include/signal.h:360,
>                  from /usr/aarch64-linux-gnu/include/sys/wait.h:30,
>                  from xenalyze.c:28:
> /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/asm/types.h:54:13: error: conflicting types for 'register_t'
>  typedef u64 register_t;
>              ^
> In file included from /usr/aarch64-linux-gnu/include/stdlib.h:314:0,
>                  from xenalyze.c:24:
> /usr/aarch64-linux-gnu/include/sys/types.h:205:13: note: previous declaration of 'register_t' was here
>  typedef int register_t __attribute__ ((__mode__ (__word__)));
>              ^

The weird thing about this one is that register_t isn't defined or used
by xenalyze at all.

The include pedigree is a bit confusing, but it looks like
aarch64-linux-gnu/include/linux/types.h is including Xen files, and that
there's a type mismatch between Xen's xen/include/asm/types.h and the
system's include/sys/types.h.

It looks like it will happen to any program which includes both #include
both stdlib.h and sys/wait.h.  If so, this is a general problem with Xen
on ARM64, not a bug in xenalyze.

(But as I said, the #include chain is a bit confusing, so I feel free to
correct me if I got something wrong...)

> In file included from xenalyze.c:33:0:
> analyze.h:19:0: error: "NR_CPUS" redefined [-Werror]
>  #define NR_CPUS 256
>  ^
> In file included from /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/xen/config.h:13:0,
>                  from /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/asm/types.h:6,
>                  from /usr/aarch64-linux-gnu/include/linux/types.h:4,
>                  from /usr/aarch64-linux-gnu/include/asm/ptrace.h:22,
>                  from /usr/aarch64-linux-gnu/include/sys/user.h:25,
>                  from /usr/aarch64-linux-gnu/include/sys/procfs.h:34,
>                  from /usr/aarch64-linux-gnu/include/sys/ucontext.h:26,
>                  from /usr/aarch64-linux-gnu/include/signal.h:360,
>                  from /usr/aarch64-linux-gnu/include/sys/wait.h:30,
>                  from xenalyze.c:28:
> /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/asm/config.h:47:0: note: this is the location of the previous definition
>  #define NR_CPUS 128
>  ^
> In file included from xenalyze.c:33:0:
> analyze.h:23:0: error: "BITS_PER_LONG" redefined [-Werror]
>  # define BITS_PER_LONG 32
>  ^
> In file included from /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/xen/config.h:13:0,
>                  from /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/asm/types.h:6,
>                  from /usr/aarch64-linux-gnu/include/linux/types.h:4,
>                  from /usr/aarch64-linux-gnu/include/asm/ptrace.h:22,
>                  from /usr/aarch64-linux-gnu/include/sys/user.h:25,
>                  from /usr/aarch64-linux-gnu/include/sys/procfs.h:34,
>                  from /usr/aarch64-linux-gnu/include/sys/ucontext.h:26,
>                  from /usr/aarch64-linux-gnu/include/signal.h:360,
>                  from /usr/aarch64-linux-gnu/include/sys/wait.h:30,
>                  from xenalyze.c:28:
> /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../xen/include/asm/config.h:23:0: note: this is the location of the previous definition
>  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
>  ^
> aarch64-linux-gnu-gcc    -o xentrace_setsize setsize.o /local/scratch/ianc/devel/committer-arm64.git/tools/xentrace/../../tools/libxc/libxenctrl.so

If including <sys/wait.h> will reliably include the Xen headers, then we
can just remove NR_CPUS and BITS_PER_LONG from analyze.h.  In the case
of NR_CPUS it would be preferable actually, since then we wouldn't need
to worry about keeping those two in sync.

But I suspect that this is a quirk of aarm64 (or perhaps of IanC's build
environment) that won't be duplicated on x86.

 -George