Re: [PATCH 05/28] kvm tools: 64-bit tidy; use PRIx64 when printf'ing u64s and link appropriately

[Matt Evans <matt@ozlabs.org>]

On 08/12/11 15:56, Matt Evans wrote:
> On 08/12/11 15:49, Ingo Molnar wrote:
>>
>> * Matt Evans <matt@ozlabs.org> wrote:
>>
>>> On 08/12/11 04:14, Pekka Enberg wrote:
>>>> On Wed, 7 Dec 2011, Ingo Molnar wrote:
>>>>
>>>>>
>>>>> * Matt Evans <matt@ozlabs.org> wrote:
>>>>>
>>>>>>>> [...]  I haven't looked closely at Matt's
>>>>>>>> patches, but it should be possible to use [un]signed long long
>>>>>>>> for the u64/s64 types, I would think.
>>>>>>>
>>>>>>> In tools/kvm/ we are using our own u64/s64 definitions, not
>>>>>>> glibc's, so i think it should be fine - as long as we don't pick
>>>>>>> up int-l64.h accidentally via the
>>>>>>> arch/powerpc/include/asm/types.h exception for user-space.
>>>>>>
>>>>>> That's what's happening here; we're __powerpc64__ and
>>>>>> !__KERNEL__, tools/kvm/include/linux/types.h includes
>>>>>> asm/types.h so gets the int-l64.h definition of __u64, as
>>>>>> above.  :/
>>>>>>
>>>>>> builtin-run.c:389: error: format `%llx' expects type `long
>>>>>> long unsigned int', but argument 2 has type `u64'
>>>>>
>>>>> So either define __KERNEL__ or patch a __NEW_USERSPACE__ define
>>>>> into power/asm/types.h and use it - if the PowerPC folks agree
>>>>> with that approach.
>>>>>
>>>>> Sane userspace should not be prevented from using the same sane
>>>>> types the kernel is already using :-)
>>>>
>>>> How does perf handle this? I'm sure it has the exact same 
>>>> issue, doesn't it?
>>>
>>> It does; ironically it uses PRIblah, so I had followed its 
>>> example.
>>
>> Sadly it regressed lately in that area - it was certainly 
>> PRIblah-less in the early stages :-)
> 
> Oh dear :-)
> 
>> Pekka, how do the headers react if we define __KERNEL__? 
>> Alternatively, allowing an override in powerpc/types.h beyond 
>> __KERNEL__ looks sensible as well.
> 
> Well, I just tried it and it ended in tears; various things bring in tons more
> (ppc) stuff from asm/, quite a few conflicts.
> 
> I've resorted to defining __KERNEL__ in linux/types.h *only* around #include
> <asm/types.h> (i.e. undefining it afterwards).  This picks up the correct
> int-ll64.h on PPC64 and doesn't break everything else.

I spoke too soon; this screws x86 (who?), in that BITS_PER_LONG's redefined as
../../include/asm-generic/bitsperlong.h now kicks in if __KERNEL__'s defined.
Defining __KERNEL__ feels a bit nasty, esp. considering these knock-on effects.

Since tools/kvm/include/linux/types.h only requires __u32, __u64 et al from
<asm/types.h>, wouldn't it be most straightforward to just #include
<asm-generic/int-ll64.h>?  This avoids #define __KERNEL__ breaking other
includes brought into userland, avoids changing system headers/distros, and
includes the file we're really interested in on both x86 & PPC.

Cheers,


Matt