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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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>





[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux