Re: [PATCH bpf 1/2] samples/bpf: Set flag __SANE_USERSPACE_TYPES__ for MIPS to fix build warnings
From: Yonghong Song
Date: Mon Jan 18 2021 - 13:51:40 EST
On 1/17/21 7:22 PM, Tiezhu Yang wrote:
On 01/14/2021 01:12 AM, Yonghong Song wrote:
On 1/13/21 2:57 AM, Tiezhu Yang wrote:
MIPS needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
'int-ll64.h' in arch/mips/include/uapi/asm/types.h and avoid compile
warnings when printing __u64 with %llu, %llx or %lld.
could you mention which command produces the following warning?
make M=samples/bpf
printf("0x%02x : %llu\n", key, value);
~~~^ ~~~~~
%lu
printf("%s/%llx;", sym->name, addr);
~~~^ ~~~~
%lx
printf(";%s %lld\n", key->waker, count);
~~~^ ~~~~~
%ld
Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
---
samples/bpf/Makefile | 4 ++++
tools/include/linux/types.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 26fc96c..27de306 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -183,6 +183,10 @@ BPF_EXTRA_CFLAGS := $(ARM_ARCH_SELECTOR)
TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
endif
+ifeq ($(ARCH), mips)
+TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
+endif
+
This change looks okay based on description in
arch/mips/include/uapi/asm/types.h
'''
/*
* We don't use int-l64.h for the kernel anymore but still use it for
* userspace to avoid code changes.
*
* However, some user programs (e.g. perf) may not want this. They can
* flag __SANE_USERSPACE_TYPES__ to get int-ll64.h here.
*/
'''
TPROGS_CFLAGS += -Wall -O2
TPROGS_CFLAGS += -Wmissing-prototypes
TPROGS_CFLAGS += -Wstrict-prototypes
diff --git a/tools/include/linux/types.h b/tools/include/linux/types.h
index 154eb4e..e9c5a21 100644
--- a/tools/include/linux/types.h
+++ b/tools/include/linux/types.h
@@ -6,7 +6,10 @@
#include <stddef.h>
#include <stdint.h>
+#ifndef __SANE_USERSPACE_TYPES__
#define __SANE_USERSPACE_TYPES__ /* For PPC64, to get LL64 types */
+#endif
What problem this patch fixed?
If add "TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__" in
samples/bpf/Makefile, it appears the following error:
Auto-detecting system features:
... libelf: [ on ]
... zlib: [ on ]
... bpf: [ OFF ]
BPF API too old
make[3]: *** [Makefile:293: bpfdep] Error 1
make[2]: *** [Makefile:156: all] Error 2
With #ifndef __SANE_USERSPACE_TYPES__ in tools/include/linux/types.h,
the above error has gone.
If this header is used, you can just
change comment from "PPC64" to "PPC64/MIPS", right?
If include <linux/types.h> in the source files which have compile warnings
when printing __u64 with %llu, %llx or %lld, it has no effect due to
actually
it includes usr/include/linux/types.h instead of
tools/include/linux/types.h,
this is because the include-directories in samples/bpf/Makefile are
searched
in the order, -I./usr/include is in the front of -I./tools/include.
So I think define __SANE_USERSPACE_TYPES__ for MIPS in samples/bpf/Makefile
is proper, at the same time, add #ifndef __SANE_USERSPACE_TYPES__ in
tools/include/linux/types.h can avoid build error and have no side effect.
I will send v2 later with mention in the commit message that this is
mips related.
It would be good if you can add the above information to the commit
message so people will know what the root cause of the issue.
If I understand correctly, if we could have include path
"tools/include" earlier than "usr/include", we might not have this
issue. The problem is that "usr/include" is preferred first (uapi)
than "tools/include" (including kernel dev headers).
I am wondering whether we could avoid changes in
tools/include/linux/types.h, e.g., by undef __SANE_USER_SPACE_TYPES
right before include
path tools/include. But that sounds like a ugly hack and actually
the change in tools/include/linux/types.h does not hurt other
compilations.
So your current change looks good to me, but please have better
explanation of the problem and why for each change in the commit
message.
Thanks,
Tiezhu
+
#include <asm/types.h>
#include <asm/posix_types.h>