Re: [PATCH bpf 1/2] samples/bpf: Set flag __SANE_USERSPACE_TYPES__ for MIPS to fix build warnings

2021-01-18 Thread Yonghong Song




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  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 
---
  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 
  #include 
  +#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  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 
  #include 





Re: [PATCH bpf 1/2] samples/bpf: Set flag __SANE_USERSPACE_TYPES__ for MIPS to fix build warnings

2021-01-17 Thread Tiezhu Yang

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  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 
---
  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 
  #include 
  +#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  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 
  #include 





Re: [PATCH bpf 1/2] samples/bpf: Set flag __SANE_USERSPACE_TYPES__ for MIPS to fix build warnings

2021-01-13 Thread Yonghong Song




On 1/13/21 2:57 AM, Tiezhu Yang wrote:

MIPS needs __SANE_USERSPACE_TYPES__ before  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 
---
  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 
  #include 
  
+#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 
  #include