* [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode
@ 2023-10-04 0:17 Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-04 0:17 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team
Fix a bunch of potentially unitialized variable usage warnings that are
reported by GCC in -O2 mode. Also silence overzealous stringop-truncation
class of warnings.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 4 +++-
.../selftests/bpf/map_tests/map_in_map_batch_ops.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/connect_ping.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/linked_list.c | 2 +-
tools/testing/selftests/bpf/prog_tests/lwt_helpers.h | 3 ++-
tools/testing/selftests/bpf/prog_tests/queue_stack_map.c | 2 +-
tools/testing/selftests/bpf/prog_tests/sockmap_basic.c | 8 ++++----
tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h | 2 +-
tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 2 +-
tools/testing/selftests/bpf/test_loader.c | 4 ++--
tools/testing/selftests/bpf/xdp_features.c | 4 ++--
tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
tools/testing/selftests/bpf/xskxceiver.c | 2 +-
15 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 47365161b6fc..a25e262dbc69 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -27,7 +27,9 @@ endif
BPF_GCC ?= $(shell command -v bpf-gcc;)
SAN_CFLAGS ?=
SAN_LDFLAGS ?= $(SAN_CFLAGS)
-CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
+CFLAGS += -g -O0 -rdynamic \
+ -Wall -Werror \
+ $(GENFLAGS) $(SAN_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
-I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
LDFLAGS += $(SAN_LDFLAGS)
diff --git a/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
index 16f1671e4bde..66191ae9863c 100644
--- a/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
@@ -33,11 +33,11 @@ static void create_inner_maps(enum bpf_map_type map_type,
{
int map_fd, map_index, ret;
__u32 map_key = 0, map_id;
- char map_name[15];
+ char map_name[16];
for (map_index = 0; map_index < OUTER_MAP_ENTRIES; map_index++) {
memset(map_name, 0, sizeof(map_name));
- sprintf(map_name, "inner_map_fd_%d", map_index);
+ snprintf(map_name, sizeof(map_name), "inner_map_fd_%d", map_index);
map_fd = bpf_map_create(map_type, map_name, sizeof(__u32),
sizeof(__u32), 1, NULL);
CHECK(map_fd < 0,
diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
index d2d9e965eba5..053f4d6da77a 100644
--- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
@@ -193,8 +193,8 @@ static int setup_progs(struct bloom_filter_map **out_skel, __u32 **out_rand_vals
void test_bloom_filter_map(void)
{
- __u32 *rand_vals, nr_rand_vals;
- struct bloom_filter_map *skel;
+ __u32 *rand_vals = NULL, nr_rand_vals = 0;
+ struct bloom_filter_map *skel = NULL;
int err;
test_fail_cases();
diff --git a/tools/testing/selftests/bpf/prog_tests/connect_ping.c b/tools/testing/selftests/bpf/prog_tests/connect_ping.c
index 289218c2216c..40fe571f2fe7 100644
--- a/tools/testing/selftests/bpf/prog_tests/connect_ping.c
+++ b/tools/testing/selftests/bpf/prog_tests/connect_ping.c
@@ -28,9 +28,9 @@ static void subtest(int cgroup_fd, struct connect_ping *skel,
.sin6_family = AF_INET6,
.sin6_addr = IN6ADDR_LOOPBACK_INIT,
};
- struct sockaddr *sa;
+ struct sockaddr *sa = NULL;
socklen_t sa_len;
- int protocol;
+ int protocol = -1;
int sock_fd;
switch (family) {
diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
index db3bf6bbe01a..69dc31383b78 100644
--- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
+++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
@@ -268,7 +268,7 @@ static struct btf *init_btf(void)
static void list_and_rb_node_same_struct(bool refcount_field)
{
- int bpf_rb_node_btf_id, bpf_refcount_btf_id, foo_btf_id;
+ int bpf_rb_node_btf_id, bpf_refcount_btf_id = 0, foo_btf_id;
struct btf *btf;
int id, err;
diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
index 61333f2a03f9..e9190574e79f 100644
--- a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
@@ -49,7 +49,8 @@ static int open_tuntap(const char *dev_name, bool need_mac)
return -1;
ifr.ifr_flags = IFF_NO_PI | (need_mac ? IFF_TAP : IFF_TUN);
- memcpy(ifr.ifr_name, dev_name, IFNAMSIZ);
+ strncpy(ifr.ifr_name, dev_name, IFNAMSIZ - 1);
+ ifr.ifr_name[IFNAMSIZ - 1] = '\0';
err = ioctl(fd, TUNSETIFF, &ifr);
if (!ASSERT_OK(err, "ioctl(TUNSETIFF)")) {
diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
index 722c5f2a7776..a043af9cd6d9 100644
--- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
@@ -14,7 +14,7 @@ static void test_queue_stack_map_by_type(int type)
int i, err, prog_fd, map_in_fd, map_out_fd;
char file[32], buf[128];
struct bpf_object *obj;
- struct iphdr iph;
+ struct iphdr iph = {};
LIBBPF_OPTS(bpf_test_run_opts, topts,
.data_in = &pkt_v4,
.data_size_in = sizeof(pkt_v4),
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 064cc5e8d9ad..2535d0653cc8 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -359,7 +359,7 @@ static void test_sockmap_progs_query(enum bpf_attach_type attach_type)
static void test_sockmap_skb_verdict_shutdown(void)
{
struct epoll_event ev, events[MAX_EVENTS];
- int n, err, map, verdict, s, c1, p1;
+ int n, err, map, verdict, s, c1 = -1, p1 = -1;
struct test_sockmap_pass_prog *skel;
int epollfd;
int zero = 0;
@@ -414,9 +414,9 @@ static void test_sockmap_skb_verdict_shutdown(void)
static void test_sockmap_skb_verdict_fionread(bool pass_prog)
{
int expected, zero = 0, sent, recvd, avail;
- int err, map, verdict, s, c0, c1, p0, p1;
- struct test_sockmap_pass_prog *pass;
- struct test_sockmap_drop_prog *drop;
+ int err, map, verdict, s, c0 = -1, c1 = -1, p0 = -1, p1 = -1;
+ struct test_sockmap_pass_prog *pass = NULL;
+ struct test_sockmap_drop_prog *drop = NULL;
char buf[256] = "0123456789";
if (pass_prog) {
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
index 36d829a65aa4..e880f97bc44d 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
@@ -378,7 +378,7 @@ static inline int enable_reuseport(int s, int progfd)
static inline int socket_loopback_reuseport(int family, int sotype, int progfd)
{
struct sockaddr_storage addr;
- socklen_t len;
+ socklen_t len = 0;
int err, s;
init_addr_loopback(family, &addr, &len);
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
index 8df8cbb447f1..e08e590b2cf8 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
@@ -73,7 +73,7 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
int family, int sotype, int mapfd)
{
struct sockaddr_storage addr;
- socklen_t len;
+ socklen_t len = 0;
u32 key = 0;
u64 value;
int err, s;
@@ -871,7 +871,7 @@ static void test_msg_redir_to_listening(struct test_sockmap_listen *skel,
static void redir_partial(int family, int sotype, int sock_map, int parser_map)
{
- int s, c0, c1, p0, p1;
+ int s, c0 = -1, c1 = -1, p0 = -1, p1 = -1;
int err, n, key, value;
char buf[] = "abc";
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index 626c461fa34d..4439ba9392f8 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -226,7 +226,7 @@ static int verify_xsk_metadata(struct xsk *xsk)
__u64 comp_addr;
void *data;
__u64 addr;
- __u32 idx;
+ __u32 idx = 0;
int ret;
ret = recvfrom(xsk_socket__fd(xsk->socket), NULL, 0, MSG_DONTWAIT, NULL, NULL);
diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index b4edd8454934..37ffa57f28a1 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -69,7 +69,7 @@ static int tester_init(struct test_loader *tester)
{
if (!tester->log_buf) {
tester->log_buf_sz = TEST_LOADER_LOG_BUF_SZ;
- tester->log_buf = malloc(tester->log_buf_sz);
+ tester->log_buf = calloc(tester->log_buf_sz, 1);
if (!ASSERT_OK_PTR(tester->log_buf, "tester_log_buf"))
return -ENOMEM;
}
@@ -538,7 +538,7 @@ void run_subtest(struct test_loader *tester,
bool unpriv)
{
struct test_subspec *subspec = unpriv ? &spec->unpriv : &spec->priv;
- struct bpf_program *tprog, *tprog_iter;
+ struct bpf_program *tprog = NULL, *tprog_iter;
struct test_spec *spec_iter;
struct cap_state caps = {};
struct bpf_object *tobj;
diff --git a/tools/testing/selftests/bpf/xdp_features.c b/tools/testing/selftests/bpf/xdp_features.c
index b449788fbd39..595c79141cf3 100644
--- a/tools/testing/selftests/bpf/xdp_features.c
+++ b/tools/testing/selftests/bpf/xdp_features.c
@@ -360,9 +360,9 @@ static int recv_msg(int sockfd, void *buf, size_t bufsize, void *val,
static int dut_run(struct xdp_features *skel)
{
int flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
- int state, err, *sockfd, ctrl_sockfd, echo_sockfd;
+ int state, err = 0, *sockfd, ctrl_sockfd, echo_sockfd;
struct sockaddr_storage ctrl_addr;
- pthread_t dut_thread;
+ pthread_t dut_thread = 0;
socklen_t addrlen;
sockfd = start_reuseport_server(AF_INET6, SOCK_STREAM, NULL,
diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
index 613321eb84c1..17c980138796 100644
--- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
+++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
@@ -234,7 +234,7 @@ static int verify_metadata(struct xsk *rx_xsk, int rxq, int server_fd, clockid_t
struct pollfd fds[rxq + 1];
__u64 comp_addr;
__u64 addr;
- __u32 idx;
+ __u32 idx = 0;
int ret;
int i;
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 43e0a5796929..b0ee1307a63b 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1023,7 +1023,7 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
pkt = pkt_stream_get_next_rx_pkt(pkt_stream, &pkts_sent);
while (pkt) {
u32 frags_processed = 0, nb_frags = 0, pkt_len = 0;
- u64 first_addr;
+ u64 first_addr = 0;
ret = gettimeofday(&tv_now, NULL);
if (ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-04 0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
@ 2023-10-04 0:17 ` Andrii Nakryiko
2023-10-04 8:27 ` Jiri Olsa
2023-10-04 0:17 ` [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field Andrii Nakryiko
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-04 0:17 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team
Add support for building selftests with -O2 level of optimization, which
allows more compiler warnings detection (like lots of potentially
uninitialized usage), but also is useful to have a faster-running test
for some CPU-intensive tests.
One can build optimized versions of libbpf and selftests by running:
$ make RELEASE=1
There is a measurable speed up of about 10 seconds for me locally,
though it's mostly capped by non-parallelized serial tests. User CPU
time goes down by total 40 seconds, from 1m10s to 0m28s.
Unoptimized build (-O0)
=======================
Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
real 1m59.937s
user 1m10.877s
sys 3m14.880s
Optimized build (-O2)
=====================
Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
real 1m50.540s
user 0m28.406s
sys 3m13.198s
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index a25e262dbc69..55d1b1848e6c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -27,7 +27,9 @@ endif
BPF_GCC ?= $(shell command -v bpf-gcc;)
SAN_CFLAGS ?=
SAN_LDFLAGS ?= $(SAN_CFLAGS)
-CFLAGS += -g -O0 -rdynamic \
+RELEASE ?=
+OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
+CFLAGS += -g $(OPT_FLAGS) -rdynamic \
-Wall -Werror \
$(GENFLAGS) $(SAN_CFLAGS) \
-I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
@@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
- EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
+ EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
cp $(RUNQSLOWER_OUTPUT)runqslower $@
@@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
$(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
$(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
- EXTRA_CFLAGS='-g -O0' \
+ EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
@@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
$(BPFOBJ) | $(BUILD_DIR)/bpftool
$(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
- EXTRA_CFLAGS='-g -O0' \
+ EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
OUTPUT=$(BUILD_DIR)/bpftool/ \
LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
@@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
$(APIDIR)/linux/bpf.h \
| $(BUILD_DIR)/libbpf
$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
- EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
+ EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
@@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
$(APIDIR)/linux/bpf.h \
| $(HOST_BUILD_DIR)/libbpf
$(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
- EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
+ EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
CC="$(HOSTCC)" LD="$(HOSTLD)" \
DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field
2023-10-04 0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
@ 2023-10-04 0:17 ` Andrii Nakryiko
2023-10-05 7:21 ` Jiri Olsa
2023-10-05 7:21 ` [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Jiri Olsa
2023-10-06 18:20 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-04 0:17 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team
We currently expect up to a three-digit number of tests and subtests, so:
#999/999: some_test/some_subtest: ...
Is the largest test/subtest we can see. If we happen to cross into
1000s, current logic will just truncate everything after 7th character.
This patch fixes this truncate and allows to go way higher (up to 31
characters in total). We still nicely align test numbers:
#60/66 core_reloc_btfgen/type_based___incompat:OK
#60/67 core_reloc_btfgen/type_based___fn_wrong_args:OK
#60/68 core_reloc_btfgen/type_id:OK
#60/69 core_reloc_btfgen/type_id___missing_targets:OK
#60/70 core_reloc_btfgen/enumval:OK
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/testing/selftests/bpf/test_progs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 4d582cac2c09..1b9387890148 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -255,7 +255,7 @@ static void print_subtest_name(int test_num, int subtest_num,
const char *test_name, char *subtest_name,
char *result)
{
- char test_num_str[TEST_NUM_WIDTH + 1];
+ char test_num_str[32];
snprintf(test_num_str, sizeof(test_num_str), "%d/%d", test_num, subtest_num);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-04 0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
@ 2023-10-04 8:27 ` Jiri Olsa
2023-10-04 17:21 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2023-10-04 8:27 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> Add support for building selftests with -O2 level of optimization, which
> allows more compiler warnings detection (like lots of potentially
> uninitialized usage), but also is useful to have a faster-running test
> for some CPU-intensive tests.
>
> One can build optimized versions of libbpf and selftests by running:
>
> $ make RELEASE=1
>
> There is a measurable speed up of about 10 seconds for me locally,
> though it's mostly capped by non-parallelized serial tests. User CPU
> time goes down by total 40 seconds, from 1m10s to 0m28s.
>
> Unoptimized build (-O0)
> =======================
> Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
>
> real 1m59.937s
> user 1m10.877s
> sys 3m14.880s
>
> Optimized build (-O2)
> =====================
> Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
>
> real 1m50.540s
> user 0m28.406s
> sys 3m13.198s
hi,
I get following error when running selftest compiled with RELEASE=1
# ./test_progs -t attach_probe/manual-legacy
test_attach_probe:PASS:skel_open 0 nsec
test_attach_probe:PASS:skel_load 0 nsec
test_attach_probe:PASS:check_bss 0 nsec
test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
test_attach_probe_manual:PASS:uprobe_offset 0 nsec
test_attach_probe_manual:PASS:attach_kprobe 0 nsec
test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
test_attach_probe_manual:PASS:attach_uprobe 0 nsec
test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
#8/2 attach_probe/manual-legacy:FAIL
#8 attach_probe:FAIL
it looks like -O2 can merge some of the trigger functions:
[root@qemu bpf]# nm test_progs | grep trigger_func
0000000000558f30 t autoattach_trigger_func.constprop.0
000000000041d240 t trigger_func
0000000000419020 t trigger_func
0000000000420e70 t trigger_func
0000000000507aa0 t trigger_func
0000000000419020 t trigger_func2
0000000000419020 t trigger_func3
0000000000419030 t trigger_func4
[root@qemu bpf]# nm test_progs | grep 0000000000419020
0000000000419020 t trigger_func
0000000000419020 t trigger_func2
0000000000419020 t trigger_func3
I got more tests fails, but I suspect it's all for similar
reason like above
jirka
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index a25e262dbc69..55d1b1848e6c 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -27,7 +27,9 @@ endif
> BPF_GCC ?= $(shell command -v bpf-gcc;)
> SAN_CFLAGS ?=
> SAN_LDFLAGS ?= $(SAN_CFLAGS)
> -CFLAGS += -g -O0 -rdynamic \
> +RELEASE ?=
> +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> -Wall -Werror \
> $(GENFLAGS) $(SAN_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
> BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
> cp $(RUNQSLOWER_OUTPUT)runqslower $@
>
> @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
> - EXTRA_CFLAGS='-g -O0' \
> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> $(BPFOBJ) | $(BUILD_DIR)/bpftool
> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
> - EXTRA_CFLAGS='-g -O0' \
> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> OUTPUT=$(BUILD_DIR)/bpftool/ \
> LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
> LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
> @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> $(APIDIR)/linux/bpf.h \
> | $(BUILD_DIR)/libbpf
> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
> DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
>
> @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> $(APIDIR)/linux/bpf.h \
> | $(HOST_BUILD_DIR)/libbpf
> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
> - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
> OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> CC="$(HOSTCC)" LD="$(HOSTLD)" \
> DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-04 8:27 ` Jiri Olsa
@ 2023-10-04 17:21 ` Andrii Nakryiko
2023-10-05 7:19 ` Jiri Olsa
0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-04 17:21 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> > Add support for building selftests with -O2 level of optimization, which
> > allows more compiler warnings detection (like lots of potentially
> > uninitialized usage), but also is useful to have a faster-running test
> > for some CPU-intensive tests.
> >
> > One can build optimized versions of libbpf and selftests by running:
> >
> > $ make RELEASE=1
> >
> > There is a measurable speed up of about 10 seconds for me locally,
> > though it's mostly capped by non-parallelized serial tests. User CPU
> > time goes down by total 40 seconds, from 1m10s to 0m28s.
> >
> > Unoptimized build (-O0)
> > =======================
> > Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
> >
> > real 1m59.937s
> > user 1m10.877s
> > sys 3m14.880s
> >
> > Optimized build (-O2)
> > =====================
> > Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
> >
> > real 1m50.540s
> > user 0m28.406s
> > sys 3m13.198s
>
> hi,
> I get following error when running selftest compiled with RELEASE=1
>
> # ./test_progs -t attach_probe/manual-legacy
> test_attach_probe:PASS:skel_open 0 nsec
> test_attach_probe:PASS:skel_load 0 nsec
> test_attach_probe:PASS:check_bss 0 nsec
> test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
> test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
> test_attach_probe_manual:PASS:uprobe_offset 0 nsec
> test_attach_probe_manual:PASS:attach_kprobe 0 nsec
> test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
> test_attach_probe_manual:PASS:attach_uprobe 0 nsec
> test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
> libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
> libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
> test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
> #8/2 attach_probe/manual-legacy:FAIL
> #8 attach_probe:FAIL
>
>
> it looks like -O2 can merge some of the trigger functions:
>
> [root@qemu bpf]# nm test_progs | grep trigger_func
> 0000000000558f30 t autoattach_trigger_func.constprop.0
> 000000000041d240 t trigger_func
> 0000000000419020 t trigger_func
> 0000000000420e70 t trigger_func
> 0000000000507aa0 t trigger_func
> 0000000000419020 t trigger_func2
> 0000000000419020 t trigger_func3
> 0000000000419030 t trigger_func4
> [root@qemu bpf]# nm test_progs | grep 0000000000419020
> 0000000000419020 t trigger_func
> 0000000000419020 t trigger_func2
> 0000000000419020 t trigger_func3
>
> I got more tests fails, but I suspect it's all for similar
> reason like above
>
yes, I didn't say that -O2 version passes all tests :) at least there
are complicated USDT cases under -O2 which libbpf can't support (if I
remember correctly, it was offset relative to global symbol case). But
it's the first step. And once we have ability to build with RELEASE=1,
we can add it as a separate test in CI and catch more of these
uninitialized usage errors. Initially we can denylist tests that are
broken due to -O2 and work to fix them.
> jirka
>
>
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> > tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index a25e262dbc69..55d1b1848e6c 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -27,7 +27,9 @@ endif
> > BPF_GCC ?= $(shell command -v bpf-gcc;)
> > SAN_CFLAGS ?=
> > SAN_LDFLAGS ?= $(SAN_CFLAGS)
> > -CFLAGS += -g -O0 -rdynamic \
> > +RELEASE ?=
> > +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> > +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> > -Wall -Werror \
> > $(GENFLAGS) $(SAN_CFLAGS) \
> > -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> > @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> > BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
> > BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
> > - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
> > cp $(RUNQSLOWER_OUTPUT)runqslower $@
> >
> > @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> > $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > - EXTRA_CFLAGS='-g -O0' \
> > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> > @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > $(BPFOBJ) | $(BUILD_DIR)/bpftool
> > $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
> > - EXTRA_CFLAGS='-g -O0' \
> > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > OUTPUT=$(BUILD_DIR)/bpftool/ \
> > LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
> > LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
> > @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > $(APIDIR)/linux/bpf.h \
> > | $(BUILD_DIR)/libbpf
> > $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> > - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
> > DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> >
> > @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > $(APIDIR)/linux/bpf.h \
> > | $(HOST_BUILD_DIR)/libbpf
> > $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
> > - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
> > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
> > OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> > --
> > 2.34.1
> >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-04 17:21 ` Andrii Nakryiko
@ 2023-10-05 7:19 ` Jiri Olsa
2023-10-05 9:04 ` Alan Maguire
0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2023-10-05 7:19 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Wed, Oct 04, 2023 at 10:21:12AM -0700, Andrii Nakryiko wrote:
> On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> > > Add support for building selftests with -O2 level of optimization, which
> > > allows more compiler warnings detection (like lots of potentially
> > > uninitialized usage), but also is useful to have a faster-running test
> > > for some CPU-intensive tests.
> > >
> > > One can build optimized versions of libbpf and selftests by running:
> > >
> > > $ make RELEASE=1
> > >
> > > There is a measurable speed up of about 10 seconds for me locally,
> > > though it's mostly capped by non-parallelized serial tests. User CPU
> > > time goes down by total 40 seconds, from 1m10s to 0m28s.
> > >
> > > Unoptimized build (-O0)
> > > =======================
> > > Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
> > >
> > > real 1m59.937s
> > > user 1m10.877s
> > > sys 3m14.880s
> > >
> > > Optimized build (-O2)
> > > =====================
> > > Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
> > >
> > > real 1m50.540s
> > > user 0m28.406s
> > > sys 3m13.198s
> >
> > hi,
> > I get following error when running selftest compiled with RELEASE=1
> >
> > # ./test_progs -t attach_probe/manual-legacy
> > test_attach_probe:PASS:skel_open 0 nsec
> > test_attach_probe:PASS:skel_load 0 nsec
> > test_attach_probe:PASS:check_bss 0 nsec
> > test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
> > test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
> > test_attach_probe_manual:PASS:uprobe_offset 0 nsec
> > test_attach_probe_manual:PASS:attach_kprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_uprobe 0 nsec
> > test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
> > libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
> > libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
> > test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
> > #8/2 attach_probe/manual-legacy:FAIL
> > #8 attach_probe:FAIL
> >
> >
> > it looks like -O2 can merge some of the trigger functions:
> >
> > [root@qemu bpf]# nm test_progs | grep trigger_func
> > 0000000000558f30 t autoattach_trigger_func.constprop.0
> > 000000000041d240 t trigger_func
> > 0000000000419020 t trigger_func
> > 0000000000420e70 t trigger_func
> > 0000000000507aa0 t trigger_func
> > 0000000000419020 t trigger_func2
> > 0000000000419020 t trigger_func3
> > 0000000000419030 t trigger_func4
> > [root@qemu bpf]# nm test_progs | grep 0000000000419020
> > 0000000000419020 t trigger_func
> > 0000000000419020 t trigger_func2
> > 0000000000419020 t trigger_func3
> >
> > I got more tests fails, but I suspect it's all for similar
> > reason like above
> >
>
> yes, I didn't say that -O2 version passes all tests :) at least there
> are complicated USDT cases under -O2 which libbpf can't support (if I
> remember correctly, it was offset relative to global symbol case). But
> it's the first step. And once we have ability to build with RELEASE=1,
> we can add it as a separate test in CI and catch more of these
> uninitialized usage errors. Initially we can denylist tests that are
> broken due to -O2 and work to fix them.
I see ;-) sounds good
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
>
> > jirka
> >
> >
> > >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index a25e262dbc69..55d1b1848e6c 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -27,7 +27,9 @@ endif
> > > BPF_GCC ?= $(shell command -v bpf-gcc;)
> > > SAN_CFLAGS ?=
> > > SAN_LDFLAGS ?= $(SAN_CFLAGS)
> > > -CFLAGS += -g -O0 -rdynamic \
> > > +RELEASE ?=
> > > +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> > > +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> > > -Wall -Werror \
> > > $(GENFLAGS) $(SAN_CFLAGS) \
> > > -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> > > @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> > > BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > > BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
> > > BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
> > > - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > > + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
> > > cp $(RUNQSLOWER_OUTPUT)runqslower $@
> > >
> > > @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > > $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> > > $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > > ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > > - EXTRA_CFLAGS='-g -O0' \
> > > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > > OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > > LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > > LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> > > @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > > $(BPFOBJ) | $(BUILD_DIR)/bpftool
> > > $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > > ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
> > > - EXTRA_CFLAGS='-g -O0' \
> > > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > > OUTPUT=$(BUILD_DIR)/bpftool/ \
> > > LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
> > > LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
> > > @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > > $(APIDIR)/linux/bpf.h \
> > > | $(BUILD_DIR)/libbpf
> > > $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> > > - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > > + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
> > > DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> > >
> > > @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > > $(APIDIR)/linux/bpf.h \
> > > | $(HOST_BUILD_DIR)/libbpf
> > > $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
> > > - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
> > > + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
> > > OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > > CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > > DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> > > --
> > > 2.34.1
> > >
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode
2023-10-04 0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field Andrii Nakryiko
@ 2023-10-05 7:21 ` Jiri Olsa
2023-10-06 18:20 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2023-10-05 7:21 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
On Tue, Oct 03, 2023 at 05:17:48PM -0700, Andrii Nakryiko wrote:
> Fix a bunch of potentially unitialized variable usage warnings that are
> reported by GCC in -O2 mode. Also silence overzealous stringop-truncation
> class of warnings.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
there's now small conflict in xskxceiver.c change on latest bpf-next/master,
but anyway looks good
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> tools/testing/selftests/bpf/Makefile | 4 +++-
> .../selftests/bpf/map_tests/map_in_map_batch_ops.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/connect_ping.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/linked_list.c | 2 +-
> tools/testing/selftests/bpf/prog_tests/lwt_helpers.h | 3 ++-
> tools/testing/selftests/bpf/prog_tests/queue_stack_map.c | 2 +-
> tools/testing/selftests/bpf/prog_tests/sockmap_basic.c | 8 ++++----
> tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h | 2 +-
> tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 2 +-
> tools/testing/selftests/bpf/test_loader.c | 4 ++--
> tools/testing/selftests/bpf/xdp_features.c | 4 ++--
> tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
> tools/testing/selftests/bpf/xskxceiver.c | 2 +-
> 15 files changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 47365161b6fc..a25e262dbc69 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -27,7 +27,9 @@ endif
> BPF_GCC ?= $(shell command -v bpf-gcc;)
> SAN_CFLAGS ?=
> SAN_LDFLAGS ?= $(SAN_CFLAGS)
> -CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \
> +CFLAGS += -g -O0 -rdynamic \
> + -Wall -Werror \
> + $(GENFLAGS) $(SAN_CFLAGS) \
> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT)
> LDFLAGS += $(SAN_LDFLAGS)
> diff --git a/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
> index 16f1671e4bde..66191ae9863c 100644
> --- a/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
> +++ b/tools/testing/selftests/bpf/map_tests/map_in_map_batch_ops.c
> @@ -33,11 +33,11 @@ static void create_inner_maps(enum bpf_map_type map_type,
> {
> int map_fd, map_index, ret;
> __u32 map_key = 0, map_id;
> - char map_name[15];
> + char map_name[16];
>
> for (map_index = 0; map_index < OUTER_MAP_ENTRIES; map_index++) {
> memset(map_name, 0, sizeof(map_name));
> - sprintf(map_name, "inner_map_fd_%d", map_index);
> + snprintf(map_name, sizeof(map_name), "inner_map_fd_%d", map_index);
> map_fd = bpf_map_create(map_type, map_name, sizeof(__u32),
> sizeof(__u32), 1, NULL);
> CHECK(map_fd < 0,
> diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> index d2d9e965eba5..053f4d6da77a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> @@ -193,8 +193,8 @@ static int setup_progs(struct bloom_filter_map **out_skel, __u32 **out_rand_vals
>
> void test_bloom_filter_map(void)
> {
> - __u32 *rand_vals, nr_rand_vals;
> - struct bloom_filter_map *skel;
> + __u32 *rand_vals = NULL, nr_rand_vals = 0;
> + struct bloom_filter_map *skel = NULL;
> int err;
>
> test_fail_cases();
> diff --git a/tools/testing/selftests/bpf/prog_tests/connect_ping.c b/tools/testing/selftests/bpf/prog_tests/connect_ping.c
> index 289218c2216c..40fe571f2fe7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/connect_ping.c
> +++ b/tools/testing/selftests/bpf/prog_tests/connect_ping.c
> @@ -28,9 +28,9 @@ static void subtest(int cgroup_fd, struct connect_ping *skel,
> .sin6_family = AF_INET6,
> .sin6_addr = IN6ADDR_LOOPBACK_INIT,
> };
> - struct sockaddr *sa;
> + struct sockaddr *sa = NULL;
> socklen_t sa_len;
> - int protocol;
> + int protocol = -1;
> int sock_fd;
>
> switch (family) {
> diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
> index db3bf6bbe01a..69dc31383b78 100644
> --- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
> +++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
> @@ -268,7 +268,7 @@ static struct btf *init_btf(void)
>
> static void list_and_rb_node_same_struct(bool refcount_field)
> {
> - int bpf_rb_node_btf_id, bpf_refcount_btf_id, foo_btf_id;
> + int bpf_rb_node_btf_id, bpf_refcount_btf_id = 0, foo_btf_id;
> struct btf *btf;
> int id, err;
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
> index 61333f2a03f9..e9190574e79f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
> +++ b/tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
> @@ -49,7 +49,8 @@ static int open_tuntap(const char *dev_name, bool need_mac)
> return -1;
>
> ifr.ifr_flags = IFF_NO_PI | (need_mac ? IFF_TAP : IFF_TUN);
> - memcpy(ifr.ifr_name, dev_name, IFNAMSIZ);
> + strncpy(ifr.ifr_name, dev_name, IFNAMSIZ - 1);
> + ifr.ifr_name[IFNAMSIZ - 1] = '\0';
>
> err = ioctl(fd, TUNSETIFF, &ifr);
> if (!ASSERT_OK(err, "ioctl(TUNSETIFF)")) {
> diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> index 722c5f2a7776..a043af9cd6d9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
> @@ -14,7 +14,7 @@ static void test_queue_stack_map_by_type(int type)
> int i, err, prog_fd, map_in_fd, map_out_fd;
> char file[32], buf[128];
> struct bpf_object *obj;
> - struct iphdr iph;
> + struct iphdr iph = {};
> LIBBPF_OPTS(bpf_test_run_opts, topts,
> .data_in = &pkt_v4,
> .data_size_in = sizeof(pkt_v4),
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index 064cc5e8d9ad..2535d0653cc8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> @@ -359,7 +359,7 @@ static void test_sockmap_progs_query(enum bpf_attach_type attach_type)
> static void test_sockmap_skb_verdict_shutdown(void)
> {
> struct epoll_event ev, events[MAX_EVENTS];
> - int n, err, map, verdict, s, c1, p1;
> + int n, err, map, verdict, s, c1 = -1, p1 = -1;
> struct test_sockmap_pass_prog *skel;
> int epollfd;
> int zero = 0;
> @@ -414,9 +414,9 @@ static void test_sockmap_skb_verdict_shutdown(void)
> static void test_sockmap_skb_verdict_fionread(bool pass_prog)
> {
> int expected, zero = 0, sent, recvd, avail;
> - int err, map, verdict, s, c0, c1, p0, p1;
> - struct test_sockmap_pass_prog *pass;
> - struct test_sockmap_drop_prog *drop;
> + int err, map, verdict, s, c0 = -1, c1 = -1, p0 = -1, p1 = -1;
> + struct test_sockmap_pass_prog *pass = NULL;
> + struct test_sockmap_drop_prog *drop = NULL;
> char buf[256] = "0123456789";
>
> if (pass_prog) {
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> index 36d829a65aa4..e880f97bc44d 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h
> @@ -378,7 +378,7 @@ static inline int enable_reuseport(int s, int progfd)
> static inline int socket_loopback_reuseport(int family, int sotype, int progfd)
> {
> struct sockaddr_storage addr;
> - socklen_t len;
> + socklen_t len = 0;
> int err, s;
>
> init_addr_loopback(family, &addr, &len);
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index 8df8cbb447f1..e08e590b2cf8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -73,7 +73,7 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
> int family, int sotype, int mapfd)
> {
> struct sockaddr_storage addr;
> - socklen_t len;
> + socklen_t len = 0;
> u32 key = 0;
> u64 value;
> int err, s;
> @@ -871,7 +871,7 @@ static void test_msg_redir_to_listening(struct test_sockmap_listen *skel,
>
> static void redir_partial(int family, int sotype, int sock_map, int parser_map)
> {
> - int s, c0, c1, p0, p1;
> + int s, c0 = -1, c1 = -1, p0 = -1, p1 = -1;
> int err, n, key, value;
> char buf[] = "abc";
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> index 626c461fa34d..4439ba9392f8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> @@ -226,7 +226,7 @@ static int verify_xsk_metadata(struct xsk *xsk)
> __u64 comp_addr;
> void *data;
> __u64 addr;
> - __u32 idx;
> + __u32 idx = 0;
> int ret;
>
> ret = recvfrom(xsk_socket__fd(xsk->socket), NULL, 0, MSG_DONTWAIT, NULL, NULL);
> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> index b4edd8454934..37ffa57f28a1 100644
> --- a/tools/testing/selftests/bpf/test_loader.c
> +++ b/tools/testing/selftests/bpf/test_loader.c
> @@ -69,7 +69,7 @@ static int tester_init(struct test_loader *tester)
> {
> if (!tester->log_buf) {
> tester->log_buf_sz = TEST_LOADER_LOG_BUF_SZ;
> - tester->log_buf = malloc(tester->log_buf_sz);
> + tester->log_buf = calloc(tester->log_buf_sz, 1);
> if (!ASSERT_OK_PTR(tester->log_buf, "tester_log_buf"))
> return -ENOMEM;
> }
> @@ -538,7 +538,7 @@ void run_subtest(struct test_loader *tester,
> bool unpriv)
> {
> struct test_subspec *subspec = unpriv ? &spec->unpriv : &spec->priv;
> - struct bpf_program *tprog, *tprog_iter;
> + struct bpf_program *tprog = NULL, *tprog_iter;
> struct test_spec *spec_iter;
> struct cap_state caps = {};
> struct bpf_object *tobj;
> diff --git a/tools/testing/selftests/bpf/xdp_features.c b/tools/testing/selftests/bpf/xdp_features.c
> index b449788fbd39..595c79141cf3 100644
> --- a/tools/testing/selftests/bpf/xdp_features.c
> +++ b/tools/testing/selftests/bpf/xdp_features.c
> @@ -360,9 +360,9 @@ static int recv_msg(int sockfd, void *buf, size_t bufsize, void *val,
> static int dut_run(struct xdp_features *skel)
> {
> int flags = XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_DRV_MODE;
> - int state, err, *sockfd, ctrl_sockfd, echo_sockfd;
> + int state, err = 0, *sockfd, ctrl_sockfd, echo_sockfd;
> struct sockaddr_storage ctrl_addr;
> - pthread_t dut_thread;
> + pthread_t dut_thread = 0;
> socklen_t addrlen;
>
> sockfd = start_reuseport_server(AF_INET6, SOCK_STREAM, NULL,
> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> index 613321eb84c1..17c980138796 100644
> --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> @@ -234,7 +234,7 @@ static int verify_metadata(struct xsk *rx_xsk, int rxq, int server_fd, clockid_t
> struct pollfd fds[rxq + 1];
> __u64 comp_addr;
> __u64 addr;
> - __u32 idx;
> + __u32 idx = 0;
> int ret;
> int i;
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 43e0a5796929..b0ee1307a63b 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -1023,7 +1023,7 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
> pkt = pkt_stream_get_next_rx_pkt(pkt_stream, &pkts_sent);
> while (pkt) {
> u32 frags_processed = 0, nb_frags = 0, pkt_len = 0;
> - u64 first_addr;
> + u64 first_addr = 0;
>
> ret = gettimeofday(&tv_now, NULL);
> if (ret)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field
2023-10-04 0:17 ` [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field Andrii Nakryiko
@ 2023-10-05 7:21 ` Jiri Olsa
0 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2023-10-05 7:21 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
On Tue, Oct 03, 2023 at 05:17:50PM -0700, Andrii Nakryiko wrote:
> We currently expect up to a three-digit number of tests and subtests, so:
>
> #999/999: some_test/some_subtest: ...
>
> Is the largest test/subtest we can see. If we happen to cross into
> 1000s, current logic will just truncate everything after 7th character.
> This patch fixes this truncate and allows to go way higher (up to 31
> characters in total). We still nicely align test numbers:
>
> #60/66 core_reloc_btfgen/type_based___incompat:OK
> #60/67 core_reloc_btfgen/type_based___fn_wrong_args:OK
> #60/68 core_reloc_btfgen/type_id:OK
> #60/69 core_reloc_btfgen/type_id___missing_targets:OK
> #60/70 core_reloc_btfgen/enumval:OK
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> tools/testing/selftests/bpf/test_progs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 4d582cac2c09..1b9387890148 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -255,7 +255,7 @@ static void print_subtest_name(int test_num, int subtest_num,
> const char *test_name, char *subtest_name,
> char *result)
> {
> - char test_num_str[TEST_NUM_WIDTH + 1];
> + char test_num_str[32];
>
> snprintf(test_num_str, sizeof(test_num_str), "%d/%d", test_num, subtest_num);
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-05 7:19 ` Jiri Olsa
@ 2023-10-05 9:04 ` Alan Maguire
2023-10-06 17:55 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Alan Maguire @ 2023-10-05 9:04 UTC (permalink / raw)
To: Jiri Olsa, Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On 05/10/2023 08:19, Jiri Olsa wrote:
> On Wed, Oct 04, 2023 at 10:21:12AM -0700, Andrii Nakryiko wrote:
>> On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>>>
>>> On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
>>>> Add support for building selftests with -O2 level of optimization, which
>>>> allows more compiler warnings detection (like lots of potentially
>>>> uninitialized usage), but also is useful to have a faster-running test
>>>> for some CPU-intensive tests.
>>>>
>>>> One can build optimized versions of libbpf and selftests by running:
>>>>
>>>> $ make RELEASE=1
>>>>
>>>> There is a measurable speed up of about 10 seconds for me locally,
>>>> though it's mostly capped by non-parallelized serial tests. User CPU
>>>> time goes down by total 40 seconds, from 1m10s to 0m28s.
>>>>
>>>> Unoptimized build (-O0)
>>>> =======================
>>>> Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
>>>>
>>>> real 1m59.937s
>>>> user 1m10.877s
>>>> sys 3m14.880s
>>>>
>>>> Optimized build (-O2)
>>>> =====================
>>>> Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
>>>>
>>>> real 1m50.540s
>>>> user 0m28.406s
>>>> sys 3m13.198s
>>>
>>> hi,
>>> I get following error when running selftest compiled with RELEASE=1
>>>
>>> # ./test_progs -t attach_probe/manual-legacy
>>> test_attach_probe:PASS:skel_open 0 nsec
>>> test_attach_probe:PASS:skel_load 0 nsec
>>> test_attach_probe:PASS:check_bss 0 nsec
>>> test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
>>> test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
>>> test_attach_probe_manual:PASS:uprobe_offset 0 nsec
>>> test_attach_probe_manual:PASS:attach_kprobe 0 nsec
>>> test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
>>> test_attach_probe_manual:PASS:attach_uprobe 0 nsec
>>> test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
>>> libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
>>> libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
>>> test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
>>> #8/2 attach_probe/manual-legacy:FAIL
>>> #8 attach_probe:FAIL
>>>
>>>
>>> it looks like -O2 can merge some of the trigger functions:
>>>
>>> [root@qemu bpf]# nm test_progs | grep trigger_func
>>> 0000000000558f30 t autoattach_trigger_func.constprop.0
>>> 000000000041d240 t trigger_func
>>> 0000000000419020 t trigger_func
>>> 0000000000420e70 t trigger_func
>>> 0000000000507aa0 t trigger_func
>>> 0000000000419020 t trigger_func2
>>> 0000000000419020 t trigger_func3
>>> 0000000000419030 t trigger_func4
>>> [root@qemu bpf]# nm test_progs | grep 0000000000419020
>>> 0000000000419020 t trigger_func
>>> 0000000000419020 t trigger_func2
>>> 0000000000419020 t trigger_func3
>>>
>>> I got more tests fails, but I suspect it's all for similar
>>> reason like above
>>>
>>
This one is an interesting failure that definitely seems worth fixing;
as above trigger_func2 and trigger_func3 were merged it looks like, and
the legacy perf_event_kprobe_open_legacy()-based attach failed due to
add_uprobe_event_legacy() failing. The latter seems to be down to the
way that gen_uprobe_legacy_event_name() constructs legacy event names
via pid, binary_path and offset. In this case it will construct the
same name for trigger_func2 and trigger_func3, hence the -EEXIST.
It _seems_ like a fix here would be to add an optional func_name to
gen_uprobe_legacy_event_name(). At least it appears that the non-legacy
attach handles this case, which is great. Regardless, we can follow
up with some of this stuff later.
>> yes, I didn't say that -O2 version passes all tests :) at least there
>> are complicated USDT cases under -O2 which libbpf can't support (if I
>> remember correctly, it was offset relative to global symbol case). But
>> it's the first step. And once we have ability to build with RELEASE=1,
>> we can add it as a separate test in CI and catch more of these
>> uninitialized usage errors. Initially we can denylist tests that are
>> broken due to -O2 and work to fix them.
>
Sounds good to me; it's a great change as we're more likely to hit
more problems that users run into.
For the series:
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> I see ;-) sounds good
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> jirka
>
>>
>>> jirka
>>>
>>>
>>>>
>>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>>> ---
>>>> tools/testing/selftests/bpf/Makefile | 14 ++++++++------
>>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>>>> index a25e262dbc69..55d1b1848e6c 100644
>>>> --- a/tools/testing/selftests/bpf/Makefile
>>>> +++ b/tools/testing/selftests/bpf/Makefile
>>>> @@ -27,7 +27,9 @@ endif
>>>> BPF_GCC ?= $(shell command -v bpf-gcc;)
>>>> SAN_CFLAGS ?=
>>>> SAN_LDFLAGS ?= $(SAN_CFLAGS)
>>>> -CFLAGS += -g -O0 -rdynamic \
>>>> +RELEASE ?=
>>>> +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
>>>> +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
>>>> -Wall -Werror \
>>>> $(GENFLAGS) $(SAN_CFLAGS) \
>>>> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
>>>> @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
>>>> BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
>>>> BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
>>>> BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
>>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
>>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
>>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
>>>> cp $(RUNQSLOWER_OUTPUT)runqslower $@
>>>>
>>>> @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
>>>> $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
>>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
>>>> ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
>>>> - EXTRA_CFLAGS='-g -O0' \
>>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
>>>> OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
>>>> LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
>>>> LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
>>>> @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
>>>> $(BPFOBJ) | $(BUILD_DIR)/bpftool
>>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
>>>> ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
>>>> - EXTRA_CFLAGS='-g -O0' \
>>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
>>>> OUTPUT=$(BUILD_DIR)/bpftool/ \
>>>> LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
>>>> LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
>>>> @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
>>>> $(APIDIR)/linux/bpf.h \
>>>> | $(BUILD_DIR)/libbpf
>>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
>>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
>>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
>>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
>>>> DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
>>>>
>>>> @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
>>>> $(APIDIR)/linux/bpf.h \
>>>> | $(HOST_BUILD_DIR)/libbpf
>>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
>>>> - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
>>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
>>>> OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
>>>> CC="$(HOSTCC)" LD="$(HOSTLD)" \
>>>> DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
>>>> --
>>>> 2.34.1
>>>>
>>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-05 9:04 ` Alan Maguire
@ 2023-10-06 17:55 ` Andrii Nakryiko
2023-10-06 17:56 ` Andrii Nakryiko
0 siblings, 1 reply; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-06 17:55 UTC (permalink / raw)
To: Alan Maguire
Cc: Jiri Olsa, Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Thu, Oct 5, 2023 at 2:04 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 05/10/2023 08:19, Jiri Olsa wrote:
> > On Wed, Oct 04, 2023 at 10:21:12AM -0700, Andrii Nakryiko wrote:
> >> On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >>>
> >>> On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> >>>> Add support for building selftests with -O2 level of optimization, which
> >>>> allows more compiler warnings detection (like lots of potentially
> >>>> uninitialized usage), but also is useful to have a faster-running test
> >>>> for some CPU-intensive tests.
> >>>>
> >>>> One can build optimized versions of libbpf and selftests by running:
> >>>>
> >>>> $ make RELEASE=1
> >>>>
> >>>> There is a measurable speed up of about 10 seconds for me locally,
> >>>> though it's mostly capped by non-parallelized serial tests. User CPU
> >>>> time goes down by total 40 seconds, from 1m10s to 0m28s.
> >>>>
> >>>> Unoptimized build (-O0)
> >>>> =======================
> >>>> Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
> >>>>
> >>>> real 1m59.937s
> >>>> user 1m10.877s
> >>>> sys 3m14.880s
> >>>>
> >>>> Optimized build (-O2)
> >>>> =====================
> >>>> Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
> >>>>
> >>>> real 1m50.540s
> >>>> user 0m28.406s
> >>>> sys 3m13.198s
> >>>
> >>> hi,
> >>> I get following error when running selftest compiled with RELEASE=1
> >>>
> >>> # ./test_progs -t attach_probe/manual-legacy
> >>> test_attach_probe:PASS:skel_open 0 nsec
> >>> test_attach_probe:PASS:skel_load 0 nsec
> >>> test_attach_probe:PASS:check_bss 0 nsec
> >>> test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
> >>> test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
> >>> test_attach_probe_manual:PASS:uprobe_offset 0 nsec
> >>> test_attach_probe_manual:PASS:attach_kprobe 0 nsec
> >>> test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
> >>> test_attach_probe_manual:PASS:attach_uprobe 0 nsec
> >>> test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
> >>> libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
> >>> libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
> >>> test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
> >>> #8/2 attach_probe/manual-legacy:FAIL
> >>> #8 attach_probe:FAIL
> >>>
> >>>
> >>> it looks like -O2 can merge some of the trigger functions:
> >>>
> >>> [root@qemu bpf]# nm test_progs | grep trigger_func
> >>> 0000000000558f30 t autoattach_trigger_func.constprop.0
> >>> 000000000041d240 t trigger_func
> >>> 0000000000419020 t trigger_func
> >>> 0000000000420e70 t trigger_func
> >>> 0000000000507aa0 t trigger_func
> >>> 0000000000419020 t trigger_func2
> >>> 0000000000419020 t trigger_func3
> >>> 0000000000419030 t trigger_func4
> >>> [root@qemu bpf]# nm test_progs | grep 0000000000419020
> >>> 0000000000419020 t trigger_func
> >>> 0000000000419020 t trigger_func2
> >>> 0000000000419020 t trigger_func3
> >>>
> >>> I got more tests fails, but I suspect it's all for similar
> >>> reason like above
> >>>
> >>
>
> This one is an interesting failure that definitely seems worth fixing;
> as above trigger_func2 and trigger_func3 were merged it looks like, and
> the legacy perf_event_kprobe_open_legacy()-based attach failed due to
> add_uprobe_event_legacy() failing. The latter seems to be down to the
> way that gen_uprobe_legacy_event_name() constructs legacy event names
> via pid, binary_path and offset. In this case it will construct the
> same name for trigger_func2 and trigger_func3, hence the -EEXIST.
>
> It _seems_ like a fix here would be to add an optional func_name to
> gen_uprobe_legacy_event_name(). At least it appears that the non-legacy
> attach handles this case, which is great. Regardless, we can follow
> up with some of this stuff later.
Yeah, let's fix this up as a follow up. I'm not sure about using
function name as part of uprobe name, mostly because these symbol
names can be super long, and I don't know what's kernel size limits.
So we probably want to keep them bounded in size. Having said that,
seems like we use binary path and we also don't sanitize that. So I
think we should try to fix all that as a follow up: sanitize and maybe
truncate binary_path, and generally make sure that each uprobe name is
unique (probably with atomic counter). This atomic static counter will
take care of all such issues, right? Maybe we should drop the binary
path from the uprobe name altogether with that?
Either way, thanks for taking a deeper look into failures!
>
> >> yes, I didn't say that -O2 version passes all tests :) at least there
> >> are complicated USDT cases under -O2 which libbpf can't support (if I
> >> remember correctly, it was offset relative to global symbol case). But
> >> it's the first step. And once we have ability to build with RELEASE=1,
> >> we can add it as a separate test in CI and catch more of these
> >> uninitialized usage errors. Initially we can denylist tests that are
> >> broken due to -O2 and work to fix them.
> >
>
> Sounds good to me; it's a great change as we're more likely to hit
> more problems that users run into.
yep
>
> For the series:
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>
> > I see ;-) sounds good
> >
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> >
> > jirka
> >
> >>
> >>> jirka
> >>>
> >>>
> >>>>
> >>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >>>> ---
> >>>> tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> >>>> 1 file changed, 8 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >>>> index a25e262dbc69..55d1b1848e6c 100644
> >>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>> @@ -27,7 +27,9 @@ endif
> >>>> BPF_GCC ?= $(shell command -v bpf-gcc;)
> >>>> SAN_CFLAGS ?=
> >>>> SAN_LDFLAGS ?= $(SAN_CFLAGS)
> >>>> -CFLAGS += -g -O0 -rdynamic \
> >>>> +RELEASE ?=
> >>>> +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> >>>> +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> >>>> -Wall -Werror \
> >>>> $(GENFLAGS) $(SAN_CFLAGS) \
> >>>> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> >>>> @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> >>>> BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> >>>> BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
> >>>> BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
> >>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> >>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
> >>>> cp $(RUNQSLOWER_OUTPUT)runqslower $@
> >>>>
> >>>> @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> >>>> $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> >>>> ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
> >>>> - EXTRA_CFLAGS='-g -O0' \
> >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> >>>> OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> >>>> LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> >>>> LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> >>>> @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> >>>> $(BPFOBJ) | $(BUILD_DIR)/bpftool
> >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> >>>> ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
> >>>> - EXTRA_CFLAGS='-g -O0' \
> >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> >>>> OUTPUT=$(BUILD_DIR)/bpftool/ \
> >>>> LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
> >>>> LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
> >>>> @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> >>>> $(APIDIR)/linux/bpf.h \
> >>>> | $(BUILD_DIR)/libbpf
> >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> >>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> >>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
> >>>> DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> >>>>
> >>>> @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> >>>> $(APIDIR)/linux/bpf.h \
> >>>> | $(HOST_BUILD_DIR)/libbpf
> >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
> >>>> - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
> >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
> >>>> OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> >>>> CC="$(HOSTCC)" LD="$(HOSTLD)" \
> >>>> DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>>
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized -O2 mode
2023-10-06 17:55 ` Andrii Nakryiko
@ 2023-10-06 17:56 ` Andrii Nakryiko
0 siblings, 0 replies; 12+ messages in thread
From: Andrii Nakryiko @ 2023-10-06 17:56 UTC (permalink / raw)
To: Alan Maguire
Cc: Jiri Olsa, Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Fri, Oct 6, 2023 at 10:55 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Oct 5, 2023 at 2:04 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > On 05/10/2023 08:19, Jiri Olsa wrote:
> > > On Wed, Oct 04, 2023 at 10:21:12AM -0700, Andrii Nakryiko wrote:
> > >> On Wed, Oct 4, 2023 at 1:27 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >>>
> > >>> On Tue, Oct 03, 2023 at 05:17:49PM -0700, Andrii Nakryiko wrote:
> > >>>> Add support for building selftests with -O2 level of optimization, which
> > >>>> allows more compiler warnings detection (like lots of potentially
> > >>>> uninitialized usage), but also is useful to have a faster-running test
> > >>>> for some CPU-intensive tests.
> > >>>>
> > >>>> One can build optimized versions of libbpf and selftests by running:
> > >>>>
> > >>>> $ make RELEASE=1
> > >>>>
> > >>>> There is a measurable speed up of about 10 seconds for me locally,
> > >>>> though it's mostly capped by non-parallelized serial tests. User CPU
> > >>>> time goes down by total 40 seconds, from 1m10s to 0m28s.
> > >>>>
> > >>>> Unoptimized build (-O0)
> > >>>> =======================
> > >>>> Summary: 430/3544 PASSED, 25 SKIPPED, 4 FAILED
> > >>>>
> > >>>> real 1m59.937s
> > >>>> user 1m10.877s
> > >>>> sys 3m14.880s
> > >>>>
> > >>>> Optimized build (-O2)
> > >>>> =====================
> > >>>> Summary: 425/3543 PASSED, 25 SKIPPED, 9 FAILED
> > >>>>
> > >>>> real 1m50.540s
> > >>>> user 0m28.406s
> > >>>> sys 3m13.198s
> > >>>
> > >>> hi,
> > >>> I get following error when running selftest compiled with RELEASE=1
> > >>>
> > >>> # ./test_progs -t attach_probe/manual-legacy
> > >>> test_attach_probe:PASS:skel_open 0 nsec
> > >>> test_attach_probe:PASS:skel_load 0 nsec
> > >>> test_attach_probe:PASS:check_bss 0 nsec
> > >>> test_attach_probe:PASS:uprobe_ref_ctr_cleanup 0 nsec
> > >>> test_attach_probe_manual:PASS:skel_kprobe_manual_open_and_load 0 nsec
> > >>> test_attach_probe_manual:PASS:uprobe_offset 0 nsec
> > >>> test_attach_probe_manual:PASS:attach_kprobe 0 nsec
> > >>> test_attach_probe_manual:PASS:attach_kretprobe 0 nsec
> > >>> test_attach_probe_manual:PASS:attach_uprobe 0 nsec
> > >>> test_attach_probe_manual:PASS:attach_uretprobe 0 nsec
> > >>> libbpf: failed to add legacy uprobe event for /proc/self/exe:0x19020: -17
> > >>> libbpf: prog 'handle_uprobe_byname': failed to create uprobe '/proc/self/exe:0x19020' perf event: File exists
> > >>> test_attach_probe_manual:FAIL:attach_uprobe_byname unexpected error: -17
> > >>> #8/2 attach_probe/manual-legacy:FAIL
> > >>> #8 attach_probe:FAIL
> > >>>
> > >>>
> > >>> it looks like -O2 can merge some of the trigger functions:
> > >>>
> > >>> [root@qemu bpf]# nm test_progs | grep trigger_func
> > >>> 0000000000558f30 t autoattach_trigger_func.constprop.0
> > >>> 000000000041d240 t trigger_func
> > >>> 0000000000419020 t trigger_func
> > >>> 0000000000420e70 t trigger_func
> > >>> 0000000000507aa0 t trigger_func
> > >>> 0000000000419020 t trigger_func2
> > >>> 0000000000419020 t trigger_func3
> > >>> 0000000000419030 t trigger_func4
> > >>> [root@qemu bpf]# nm test_progs | grep 0000000000419020
> > >>> 0000000000419020 t trigger_func
> > >>> 0000000000419020 t trigger_func2
> > >>> 0000000000419020 t trigger_func3
> > >>>
> > >>> I got more tests fails, but I suspect it's all for similar
> > >>> reason like above
> > >>>
> > >>
> >
> > This one is an interesting failure that definitely seems worth fixing;
> > as above trigger_func2 and trigger_func3 were merged it looks like, and
> > the legacy perf_event_kprobe_open_legacy()-based attach failed due to
> > add_uprobe_event_legacy() failing. The latter seems to be down to the
> > way that gen_uprobe_legacy_event_name() constructs legacy event names
> > via pid, binary_path and offset. In this case it will construct the
> > same name for trigger_func2 and trigger_func3, hence the -EEXIST.
> >
> > It _seems_ like a fix here would be to add an optional func_name to
> > gen_uprobe_legacy_event_name(). At least it appears that the non-legacy
> > attach handles this case, which is great. Regardless, we can follow
> > up with some of this stuff later.
>
> Yeah, let's fix this up as a follow up. I'm not sure about using
> function name as part of uprobe name, mostly because these symbol
> names can be super long, and I don't know what's kernel size limits.
> So we probably want to keep them bounded in size. Having said that,
> seems like we use binary path and we also don't sanitize that. So I
Scratch that, we do sanitization at the end (I don't know how I missed
it the first time), so the concern is only about the potentially very
large binary path length.
I think either way we should have a global counter for all uprobes to
avoid any probability of a naming conflict.
> think we should try to fix all that as a follow up: sanitize and maybe
> truncate binary_path, and generally make sure that each uprobe name is
> unique (probably with atomic counter). This atomic static counter will
> take care of all such issues, right? Maybe we should drop the binary
> path from the uprobe name altogether with that?
>
> Either way, thanks for taking a deeper look into failures!
>
> >
> > >> yes, I didn't say that -O2 version passes all tests :) at least there
> > >> are complicated USDT cases under -O2 which libbpf can't support (if I
> > >> remember correctly, it was offset relative to global symbol case). But
> > >> it's the first step. And once we have ability to build with RELEASE=1,
> > >> we can add it as a separate test in CI and catch more of these
> > >> uninitialized usage errors. Initially we can denylist tests that are
> > >> broken due to -O2 and work to fix them.
> > >
> >
> > Sounds good to me; it's a great change as we're more likely to hit
> > more problems that users run into.
>
> yep
>
> >
> > For the series:
> >
> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> >
> > > I see ;-) sounds good
> > >
> > > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > >
> > > jirka
> > >
> > >>
> > >>> jirka
> > >>>
> > >>>
> > >>>>
> > >>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > >>>> ---
> > >>>> tools/testing/selftests/bpf/Makefile | 14 ++++++++------
> > >>>> 1 file changed, 8 insertions(+), 6 deletions(-)
> > >>>>
> > >>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > >>>> index a25e262dbc69..55d1b1848e6c 100644
> > >>>> --- a/tools/testing/selftests/bpf/Makefile
> > >>>> +++ b/tools/testing/selftests/bpf/Makefile
> > >>>> @@ -27,7 +27,9 @@ endif
> > >>>> BPF_GCC ?= $(shell command -v bpf-gcc;)
> > >>>> SAN_CFLAGS ?=
> > >>>> SAN_LDFLAGS ?= $(SAN_CFLAGS)
> > >>>> -CFLAGS += -g -O0 -rdynamic \
> > >>>> +RELEASE ?=
> > >>>> +OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0)
> > >>>> +CFLAGS += -g $(OPT_FLAGS) -rdynamic \
> > >>>> -Wall -Werror \
> > >>>> $(GENFLAGS) $(SAN_CFLAGS) \
> > >>>> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
> > >>>> @@ -241,7 +243,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT)
> > >>>> BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > >>>> BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \
> > >>>> BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \
> > >>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > >>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
> > >>>> cp $(RUNQSLOWER_OUTPUT)runqslower $@
> > >>>>
> > >>>> @@ -279,7 +281,7 @@ $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > >>>> $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/bpftool
> > >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > >>>> ARCH= CROSS_COMPILE= CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > >>>> - EXTRA_CFLAGS='-g -O0' \
> > >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > >>>> OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \
> > >>>> LIBBPF_OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > >>>> LIBBPF_DESTDIR=$(HOST_SCRATCH_DIR)/ \
> > >>>> @@ -290,7 +292,7 @@ $(CROSS_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > >>>> $(BPFOBJ) | $(BUILD_DIR)/bpftool
> > >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFTOOLDIR) \
> > >>>> ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) \
> > >>>> - EXTRA_CFLAGS='-g -O0' \
> > >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' \
> > >>>> OUTPUT=$(BUILD_DIR)/bpftool/ \
> > >>>> LIBBPF_OUTPUT=$(BUILD_DIR)/libbpf/ \
> > >>>> LIBBPF_DESTDIR=$(SCRATCH_DIR)/ \
> > >>>> @@ -313,7 +315,7 @@ $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > >>>> $(APIDIR)/linux/bpf.h \
> > >>>> | $(BUILD_DIR)/libbpf
> > >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) OUTPUT=$(BUILD_DIR)/libbpf/ \
> > >>>> - EXTRA_CFLAGS='-g -O0 $(SAN_CFLAGS)' \
> > >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \
> > >>>> EXTRA_LDFLAGS='$(SAN_LDFLAGS)' \
> > >>>> DESTDIR=$(SCRATCH_DIR) prefix= all install_headers
> > >>>>
> > >>>> @@ -322,7 +324,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile) \
> > >>>> $(APIDIR)/linux/bpf.h \
> > >>>> | $(HOST_BUILD_DIR)/libbpf
> > >>>> $(Q)$(MAKE) $(submake_extras) -C $(BPFDIR) \
> > >>>> - EXTRA_CFLAGS='-g -O0' ARCH= CROSS_COMPILE= \
> > >>>> + EXTRA_CFLAGS='-g $(OPT_FLAGS)' ARCH= CROSS_COMPILE= \
> > >>>> OUTPUT=$(HOST_BUILD_DIR)/libbpf/ \
> > >>>> CC="$(HOSTCC)" LD="$(HOSTLD)" \
> > >>>> DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
> > >>>> --
> > >>>> 2.34.1
> > >>>>
> > >>>>
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode
2023-10-04 0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
` (2 preceding siblings ...)
2023-10-05 7:21 ` [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Jiri Olsa
@ 2023-10-06 18:20 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-06 18:20 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Tue, 3 Oct 2023 17:17:48 -0700 you wrote:
> Fix a bunch of potentially unitialized variable usage warnings that are
> reported by GCC in -O2 mode. Also silence overzealous stringop-truncation
> class of warnings.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> tools/testing/selftests/bpf/Makefile | 4 +++-
> .../selftests/bpf/map_tests/map_in_map_batch_ops.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/connect_ping.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/linked_list.c | 2 +-
> tools/testing/selftests/bpf/prog_tests/lwt_helpers.h | 3 ++-
> tools/testing/selftests/bpf/prog_tests/queue_stack_map.c | 2 +-
> tools/testing/selftests/bpf/prog_tests/sockmap_basic.c | 8 ++++----
> tools/testing/selftests/bpf/prog_tests/sockmap_helpers.h | 2 +-
> tools/testing/selftests/bpf/prog_tests/sockmap_listen.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 2 +-
> tools/testing/selftests/bpf/test_loader.c | 4 ++--
> tools/testing/selftests/bpf/xdp_features.c | 4 ++--
> tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
> tools/testing/selftests/bpf/xskxceiver.c | 2 +-
> 15 files changed, 27 insertions(+), 24 deletions(-)
Here is the summary with links:
- [bpf-next,1/3] selftests/bpf: fix compiler warnings reported in -O2 mode
(no matching commit)
- [bpf-next,2/3] selftests/bpf: support building selftests in optimized -O2 mode
https://git.kernel.org/bpf/bpf-next/c/46475cc0dded
- [bpf-next,3/3] selftests/bpf: don't truncate #test/subtest field
https://git.kernel.org/bpf/bpf-next/c/0af3aace5b91
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-10-06 18:20 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-04 0:17 [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 2/3] selftests/bpf: support building selftests in optimized " Andrii Nakryiko
2023-10-04 8:27 ` Jiri Olsa
2023-10-04 17:21 ` Andrii Nakryiko
2023-10-05 7:19 ` Jiri Olsa
2023-10-05 9:04 ` Alan Maguire
2023-10-06 17:55 ` Andrii Nakryiko
2023-10-06 17:56 ` Andrii Nakryiko
2023-10-04 0:17 ` [PATCH bpf-next 3/3] selftests/bpf: don't truncate #test/subtest field Andrii Nakryiko
2023-10-05 7:21 ` Jiri Olsa
2023-10-05 7:21 ` [PATCH bpf-next 1/3] selftests/bpf: fix compiler warnings reported in -O2 mode Jiri Olsa
2023-10-06 18:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).