Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

[Alexei Starovoitov]

On 11/23/17 2:02 AM, Peter Zijlstra wrote:
> On Wed, Nov 15, 2017 at 09:23:33AM -0800, Song Liu wrote:
>
> > Note: We use type __u64 for pointer probe_desc instead of __aligned_u64.
> > The reason here is to avoid changing the size of struct perf_event_attr,
> > and breaking new-kernel-old-utility scenario. To avoid alignment problem
> > with the pointer, we will (in the following patches) copy probe_desc to
> > __aligned_u64 before using it as pointer.
>
> ISTR there are only relatively few architectures where __u64 and
> __aligned_u64 are not the same thing.
>
> The comment that goes with it seems to suggest i386 has short alignment
> for u64 but my compiler says differently:
>
>         printf("%d, %d\n", sizeof(unsigned long long), __alignof__(unsigned long long));
>
> $ gcc -m32 -o align align.c && ./align
> 8, 8

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include <stdio.h>

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
        printf("%d, %d\n", sizeof(unsigned long long),
               __alignof__(unsigned long long));
        printf("%d, %d\n", sizeof(s), __alignof__(s));
        printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4

so we have to use __aligned_u64 in uapi.

Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.
If you prefer we can do the same around 'config1', but it's not
any prettier.
We considered adding __aligned_u64 to the end of
'struct perf_event_attr', but it's a waste for most users, so reusing
the space of 'config' field like this seems the least evil.