[bpf-next,v4,04/10] tools/runqslower: Use consistent include paths for libbpf
diff mbox series

Message ID 157926820131.1555735.1177228853838027248.stgit@toke.dk
State New
Headers show
Series
  • tools: Use consistent libbpf include paths everywhere
Related show

Commit Message

Toke Høiland-Jørgensen Jan. 17, 2020, 1:36 p.m. UTC
From: Toke Høiland-Jørgensen <toke@redhat.com>

Fix the runqslower tool to include libbpf header files with the bpf/
prefix, to be consistent with external users of the library. Also ensure
that all includes of exported libbpf header files (those that are exported
on 'make install' of the library) use bracketed includes instead of quoted.

To not break the build, keep the old include path until everything has been
changed to the new one; a subsequent patch will remove that.

Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/bpf/runqslower/Makefile         |    5 +++--
 tools/bpf/runqslower/runqslower.bpf.c |    2 +-
 tools/bpf/runqslower/runqslower.c     |    4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

Comments

Andrii Nakryiko Jan. 17, 2020, 9:51 p.m. UTC | #1
On Fri, Jan 17, 2020 at 5:37 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> Fix the runqslower tool to include libbpf header files with the bpf/
> prefix, to be consistent with external users of the library. Also ensure
> that all includes of exported libbpf header files (those that are exported
> on 'make install' of the library) use bracketed includes instead of quoted.
>
> To not break the build, keep the old include path until everything has been
> changed to the new one; a subsequent patch will remove that.
>
> Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  tools/bpf/runqslower/Makefile         |    5 +++--
>  tools/bpf/runqslower/runqslower.bpf.c |    2 +-
>  tools/bpf/runqslower/runqslower.c     |    4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> index b62fc9646c39..9f022f7f2593 100644
> --- a/tools/bpf/runqslower/Makefile
> +++ b/tools/bpf/runqslower/Makefile
> @@ -5,6 +5,7 @@ LLC := llc
>  LLVM_STRIP := llvm-strip
>  DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
>  BPFTOOL ?= $(DEFAULT_BPFTOOL)
> +INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(abspath ../../lib/bpf)
>  LIBBPF_SRC := $(abspath ../../lib/bpf)

drop LIBBPF_SRC, it's not used anymore

>  CFLAGS := -g -Wall
>
> @@ -51,13 +52,13 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
>
>  $(OUTPUT)/%.bpf.o: %.bpf.c $(OUTPUT)/libbpf.a | $(OUTPUT)
>         $(call msg,BPF,$@)
> -       $(Q)$(CLANG) -g -O2 -target bpf -I$(OUTPUT) -I$(LIBBPF_SRC)           \
> +       $(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)           \

please preserve formatting and alignment conventions of a file

>                  -c $(filter %.c,$^) -o $@ &&                                 \
>         $(LLVM_STRIP) -g $@
>
>  $(OUTPUT)/%.o: %.c | $(OUTPUT)
>         $(call msg,CC,$@)
> -       $(Q)$(CC) $(CFLAGS) -I$(LIBBPF_SRC) -I$(OUTPUT) -c $(filter %.c,$^) -o $@
> +       $(Q)$(CC) $(CFLAGS) $(INCLUDES) -c $(filter %.c,$^) -o $@
>
>  $(OUTPUT):

[...]
Toke Høiland-Jørgensen Jan. 20, 2020, 12:56 p.m. UTC | #2
Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Fri, Jan 17, 2020 at 5:37 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> Fix the runqslower tool to include libbpf header files with the bpf/
>> prefix, to be consistent with external users of the library. Also ensure
>> that all includes of exported libbpf header files (those that are exported
>> on 'make install' of the library) use bracketed includes instead of quoted.
>>
>> To not break the build, keep the old include path until everything has been
>> changed to the new one; a subsequent patch will remove that.
>>
>> Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>>  tools/bpf/runqslower/Makefile         |    5 +++--
>>  tools/bpf/runqslower/runqslower.bpf.c |    2 +-
>>  tools/bpf/runqslower/runqslower.c     |    4 ++--
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
>> index b62fc9646c39..9f022f7f2593 100644
>> --- a/tools/bpf/runqslower/Makefile
>> +++ b/tools/bpf/runqslower/Makefile
>> @@ -5,6 +5,7 @@ LLC := llc
>>  LLVM_STRIP := llvm-strip
>>  DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
>>  BPFTOOL ?= $(DEFAULT_BPFTOOL)
>> +INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(abspath ../../lib/bpf)
>>  LIBBPF_SRC := $(abspath ../../lib/bpf)
>
> drop LIBBPF_SRC, it's not used anymore

It is: in the rule for building libbpf there's a '-C $(LIBBPF_SRC)'

-Toke
Andrii Nakryiko Jan. 20, 2020, 6:35 p.m. UTC | #3
On Mon, Jan 20, 2020 at 4:57 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>
> > On Fri, Jan 17, 2020 at 5:37 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >>
> >> Fix the runqslower tool to include libbpf header files with the bpf/
> >> prefix, to be consistent with external users of the library. Also ensure
> >> that all includes of exported libbpf header files (those that are exported
> >> on 'make install' of the library) use bracketed includes instead of quoted.
> >>
> >> To not break the build, keep the old include path until everything has been
> >> changed to the new one; a subsequent patch will remove that.
> >>
> >> Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
> >> Acked-by: Andrii Nakryiko <andriin@fb.com>
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >> ---
> >>  tools/bpf/runqslower/Makefile         |    5 +++--
> >>  tools/bpf/runqslower/runqslower.bpf.c |    2 +-
> >>  tools/bpf/runqslower/runqslower.c     |    4 ++--
> >>  3 files changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> >> index b62fc9646c39..9f022f7f2593 100644
> >> --- a/tools/bpf/runqslower/Makefile
> >> +++ b/tools/bpf/runqslower/Makefile
> >> @@ -5,6 +5,7 @@ LLC := llc
> >>  LLVM_STRIP := llvm-strip
> >>  DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
> >>  BPFTOOL ?= $(DEFAULT_BPFTOOL)
> >> +INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(abspath ../../lib/bpf)
> >>  LIBBPF_SRC := $(abspath ../../lib/bpf)
> >
> > drop LIBBPF_SRC, it's not used anymore
>
> It is: in the rule for building libbpf there's a '-C $(LIBBPF_SRC)'
>

Ah, right, missed that one. Looked a bit weird to have $(abspath
../../lib/bpf) used in INCLUDES and then separate LIBBPF_SRC
definition there, maybe

LIBBPF_SRC := $(abspath ../../lib/bpf)
INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(LIBBPF_SRC)

> -Toke
>
Andrii Nakryiko Jan. 20, 2020, 9 p.m. UTC | #4
On Mon, Jan 20, 2020 at 10:35 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jan 20, 2020 at 4:57 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> >
> > > On Fri, Jan 17, 2020 at 5:37 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >>
> > >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> > >>
> > >> Fix the runqslower tool to include libbpf header files with the bpf/
> > >> prefix, to be consistent with external users of the library. Also ensure
> > >> that all includes of exported libbpf header files (those that are exported
> > >> on 'make install' of the library) use bracketed includes instead of quoted.
> > >>
> > >> To not break the build, keep the old include path until everything has been
> > >> changed to the new one; a subsequent patch will remove that.
> > >>
> > >> Fixes: 6910d7d3867a ("selftests/bpf: Ensure bpf_helper_defs.h are taken from selftests dir")
> > >> Acked-by: Andrii Nakryiko <andriin@fb.com>
> > >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > >> ---
> > >>  tools/bpf/runqslower/Makefile         |    5 +++--
> > >>  tools/bpf/runqslower/runqslower.bpf.c |    2 +-
> > >>  tools/bpf/runqslower/runqslower.c     |    4 ++--
> > >>  3 files changed, 6 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> > >> index b62fc9646c39..9f022f7f2593 100644
> > >> --- a/tools/bpf/runqslower/Makefile
> > >> +++ b/tools/bpf/runqslower/Makefile
> > >> @@ -5,6 +5,7 @@ LLC := llc
> > >>  LLVM_STRIP := llvm-strip
> > >>  DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
> > >>  BPFTOOL ?= $(DEFAULT_BPFTOOL)
> > >> +INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(abspath ../../lib/bpf)
> > >>  LIBBPF_SRC := $(abspath ../../lib/bpf)
> > >
> > > drop LIBBPF_SRC, it's not used anymore
> >
> > It is: in the rule for building libbpf there's a '-C $(LIBBPF_SRC)'
> >
>
> Ah, right, missed that one. Looked a bit weird to have $(abspath
> ../../lib/bpf) used in INCLUDES and then separate LIBBPF_SRC
> definition there, maybe
>
> LIBBPF_SRC := $(abspath ../../lib/bpf)
> INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(LIBBPF_SRC)

Never mind, you delete it in later patch anyway.

>
> > -Toke
> >

Patch
diff mbox series

diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
index b62fc9646c39..9f022f7f2593 100644
--- a/tools/bpf/runqslower/Makefile
+++ b/tools/bpf/runqslower/Makefile
@@ -5,6 +5,7 @@  LLC := llc
 LLVM_STRIP := llvm-strip
 DEFAULT_BPFTOOL := $(OUTPUT)/sbin/bpftool
 BPFTOOL ?= $(DEFAULT_BPFTOOL)
+INCLUDES := -I$(OUTPUT) -I$(abspath ../../lib) -I$(abspath ../../lib/bpf)
 LIBBPF_SRC := $(abspath ../../lib/bpf)
 CFLAGS := -g -Wall
 
@@ -51,13 +52,13 @@  $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
 
 $(OUTPUT)/%.bpf.o: %.bpf.c $(OUTPUT)/libbpf.a | $(OUTPUT)
 	$(call msg,BPF,$@)
-	$(Q)$(CLANG) -g -O2 -target bpf -I$(OUTPUT) -I$(LIBBPF_SRC)	      \
+	$(Q)$(CLANG) -g -O2 -target bpf $(INCLUDES)	      \
 		 -c $(filter %.c,$^) -o $@ &&				      \
 	$(LLVM_STRIP) -g $@
 
 $(OUTPUT)/%.o: %.c | $(OUTPUT)
 	$(call msg,CC,$@)
-	$(Q)$(CC) $(CFLAGS) -I$(LIBBPF_SRC) -I$(OUTPUT) -c $(filter %.c,$^) -o $@
+	$(Q)$(CC) $(CFLAGS) $(INCLUDES) -c $(filter %.c,$^) -o $@
 
 $(OUTPUT):
 	$(call msg,MKDIR,$@)
diff --git a/tools/bpf/runqslower/runqslower.bpf.c b/tools/bpf/runqslower/runqslower.bpf.c
index 623cce4d37f5..48a39f72fadf 100644
--- a/tools/bpf/runqslower/runqslower.bpf.c
+++ b/tools/bpf/runqslower/runqslower.bpf.c
@@ -1,7 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2019 Facebook
 #include "vmlinux.h"
-#include <bpf_helpers.h>
+#include <bpf/bpf_helpers.h>
 #include "runqslower.h"
 
 #define TASK_RUNNING 0
diff --git a/tools/bpf/runqslower/runqslower.c b/tools/bpf/runqslower/runqslower.c
index 996f0e2c560e..d89715844952 100644
--- a/tools/bpf/runqslower/runqslower.c
+++ b/tools/bpf/runqslower/runqslower.c
@@ -6,8 +6,8 @@ 
 #include <string.h>
 #include <sys/resource.h>
 #include <time.h>
-#include <libbpf.h>
-#include <bpf.h>
+#include <bpf/libbpf.h>
+#include <bpf/bpf.h>
 #include "runqslower.h"
 #include "runqslower.skel.h"