| Message ID | 1610535453-2352-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Fix build errors and warnings when make M=samples/bpf | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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? > > 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@loongson.cn> > --- > 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 this header is used, you can just change comment from "PPC64" to "PPC64/MIPS", right? > + > #include <asm/types.h> > #include <asm/posix_types.h> > >
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@loongson.cn> >> --- >> 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. Thanks, Tiezhu > >> + >> #include <asm/types.h> >> #include <asm/posix_types.h> >>
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@loongson.cn> >>> --- >>> 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> >>> >
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 + 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 + #include <asm/types.h> #include <asm/posix_types.h>
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. 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@loongson.cn> --- samples/bpf/Makefile | 4 ++++ tools/include/linux/types.h | 3 +++ 2 files changed, 7 insertions(+)