* [PATCH v1 1/7] perf llvm: Fix inadvertent file creation
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
@ 2023-01-10 22:19 ` Ian Rogers
2023-01-10 22:19 ` [PATCH v1 2/7] tools lib: Move strbuf to libapi Ian Rogers
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
The LLVM template is first echo-ed into command_out and then
command_out executed. The echo surrounds the template with double
quotes, however, the template itself may contain quotes. This is
generally innocuous but in tools/perf/tests/bpf-script-test-prologue.c
we see:
...
SEC("func=null_lseek file->f_mode offset orig")
...
where the first double quote ends the double quote of the echo, then
the > redirects output into a file called f_mode.
To avoid this inadvertent behavior substitute redirects and similar
characters to be ASCII control codes, then substitute the output in
the echo back again.
Fixes: 5eab5a7ee032 ("perf llvm: Display eBPF compiling command in debug output")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/llvm-utils.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 650ffe336f3a..4e8e243a6e4b 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -531,14 +531,37 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
pr_debug("llvm compiling command template: %s\n", template);
+ /*
+ * Below, substitute control characters for values that can cause the
+ * echo to misbehave, then substitute the values back.
+ */
err = -ENOMEM;
- if (asprintf(&command_echo, "echo -n \"%s\"", template) < 0)
+ if (asprintf(&command_echo, "echo -n \a%s\a", template) < 0)
goto errout;
+#define SWAP_CHAR(a, b) do { if (*p == a) *p = b; } while (0)
+ for (char *p = command_echo; *p; p++) {
+ SWAP_CHAR('<', '\001');
+ SWAP_CHAR('>', '\002');
+ SWAP_CHAR('"', '\003');
+ SWAP_CHAR('\'', '\004');
+ SWAP_CHAR('|', '\005');
+ SWAP_CHAR('&', '\006');
+ SWAP_CHAR('\a', '"');
+ }
err = read_from_pipe(command_echo, (void **) &command_out, NULL);
if (err)
goto errout;
+ for (char *p = command_out; *p; p++) {
+ SWAP_CHAR('\001', '<');
+ SWAP_CHAR('\002', '>');
+ SWAP_CHAR('\003', '"');
+ SWAP_CHAR('\004', '\'');
+ SWAP_CHAR('\005', '|');
+ SWAP_CHAR('\006', '&');
+ }
+#undef SWAP_CHAR
pr_debug("llvm compiling command : %s\n", command_out);
err = read_from_pipe(template, &obj_buf, &obj_buf_sz);
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 2/7] tools lib: Move strbuf to libapi
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
2023-01-10 22:19 ` [PATCH v1 1/7] perf llvm: Fix inadvertent file creation Ian Rogers
@ 2023-01-10 22:19 ` Ian Rogers
2023-01-10 22:19 ` [PATCH v1 3/7] tools lib subcmd: Add run_command_strbuf Ian Rogers
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
Move strbuf, appendable C strings, to libapi so that other libraries
may use it.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/lib/api/Build | 1 +
tools/lib/api/Makefile | 2 +-
tools/{perf/util => lib/api}/strbuf.c | 5 +++--
tools/{perf/util => lib/api}/strbuf.h | 0
tools/perf/bench/evlist-open-close.c | 2 +-
tools/perf/builtin-help.c | 2 +-
tools/perf/builtin-list.c | 2 +-
tools/perf/util/Build | 1 -
tools/perf/util/cache.h | 2 +-
tools/perf/util/dwarf-aux.c | 2 +-
tools/perf/util/env.c | 2 +-
| 2 +-
tools/perf/util/metricgroup.c | 2 +-
tools/perf/util/pfm.c | 2 +-
tools/perf/util/pmu.c | 2 +-
tools/perf/util/probe-event.c | 2 +-
tools/perf/util/probe-file.c | 2 +-
tools/perf/util/probe-finder.c | 2 +-
tools/perf/util/sort.c | 2 +-
19 files changed, 19 insertions(+), 18 deletions(-)
rename tools/{perf/util => lib/api}/strbuf.c (97%)
rename tools/{perf/util => lib/api}/strbuf.h (100%)
diff --git a/tools/lib/api/Build b/tools/lib/api/Build
index 6e2373db5598..2eab5abbad50 100644
--- a/tools/lib/api/Build
+++ b/tools/lib/api/Build
@@ -3,6 +3,7 @@ libapi-y += fs/
libapi-y += cpu.o
libapi-y += debug.o
libapi-y += str_error_r.o
+libapi-y += strbuf.o
$(OUTPUT)str_error_r.o: ../str_error_r.c FORCE
$(call rule_mkdir)
diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
index 044860ac1ed1..dc2d810dfbad 100644
--- a/tools/lib/api/Makefile
+++ b/tools/lib/api/Makefile
@@ -99,7 +99,7 @@ install_lib: $(LIBFILE)
$(call do_install_mkdir,$(libdir_SQ)); \
cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
-HDRS := cpu.h debug.h io.h
+HDRS := cpu.h debug.h io.h strbuf.h
FD_HDRS := fd/array.h
FS_HDRS := fs/fs.h fs/tracing_path.h
INSTALL_HDRS_PFX := $(DESTDIR)$(prefix)/include/api
diff --git a/tools/perf/util/strbuf.c b/tools/lib/api/strbuf.c
similarity index 97%
rename from tools/perf/util/strbuf.c
rename to tools/lib/api/strbuf.c
index a64a37628f12..4639b2d02e62 100644
--- a/tools/perf/util/strbuf.c
+++ b/tools/lib/api/strbuf.c
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-#include "cache.h"
-#include "debug.h"
+#include "debug-internal.h"
#include "strbuf.h"
#include <linux/kernel.h>
#include <linux/string.h>
@@ -43,6 +42,7 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)
return res;
}
+#define alloc_nr(x) (((x)+16)*3/2)
int strbuf_grow(struct strbuf *sb, size_t extra)
{
char *buf;
@@ -69,6 +69,7 @@ int strbuf_grow(struct strbuf *sb, size_t extra)
sb->alloc = nr;
return 0;
}
+#undef alloc_nr
int strbuf_addch(struct strbuf *sb, int c)
{
diff --git a/tools/perf/util/strbuf.h b/tools/lib/api/strbuf.h
similarity index 100%
rename from tools/perf/util/strbuf.h
rename to tools/lib/api/strbuf.h
diff --git a/tools/perf/bench/evlist-open-close.c b/tools/perf/bench/evlist-open-close.c
index 5a27691469ed..d8a8fcadb9ca 100644
--- a/tools/perf/bench/evlist-open-close.c
+++ b/tools/perf/bench/evlist-open-close.c
@@ -8,7 +8,7 @@
#include "../util/stat.h"
#include "../util/evlist.h"
#include "../util/evsel.h"
-#include "../util/strbuf.h"
+#include <api/strbuf.h>
#include "../util/record.h"
#include "../util/parse-events.h"
#include "internal/threadmap.h"
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 3976aebe3677..8874e1e0335b 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -6,7 +6,7 @@
*/
#include "util/cache.h"
#include "util/config.h"
-#include "util/strbuf.h"
+#include <api/strbuf.h>
#include "builtin.h"
#include <subcmd/exec-cmd.h>
#include "common-cmds.h"
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 137d73edb541..ca52227f311c 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -17,7 +17,7 @@
#include "util/metricgroup.h"
#include "util/string2.h"
#include "util/strlist.h"
-#include "util/strbuf.h"
+#include <api/strbuf.h>
#include <subcmd/pager.h>
#include <subcmd/parse-options.h>
#include <linux/zalloc.h>
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 79b9498886a2..5c68ab8c69f8 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -37,7 +37,6 @@ perf-y += libstring.o
perf-y += bitmap.o
perf-y += hweight.o
perf-y += smt.o
-perf-y += strbuf.o
perf-y += string.o
perf-y += strlist.o
perf-y += strfilter.o
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 9f2e36ef5072..19e60decb24c 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -2,7 +2,7 @@
#ifndef __PERF_CACHE_H
#define __PERF_CACHE_H
-#include "strbuf.h"
+#include <api/strbuf.h>
#include <subcmd/pager.h>
#include "../ui/ui.h"
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index b07414409771..673ddfeb938d 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -9,7 +9,7 @@
#include <stdlib.h>
#include "debug.h"
#include "dwarf-aux.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "string2.h"
/**
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 5b8cf6a421a4..3dc1c51a8335 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -10,7 +10,7 @@
#include <sys/utsname.h>
#include <stdlib.h>
#include <string.h>
-#include "strbuf.h"
+#include <api/strbuf.h>
struct perf_env perf_env;
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 404d816ca124..35067c22a47f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -38,7 +38,7 @@
#include "cpumap.h"
#include "pmu.h"
#include "vdso.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "build-id.h"
#include "data.h"
#include <api/fs/fs.h>
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b9c273ed080a..d1d21605715a 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -9,7 +9,7 @@
#include "debug.h"
#include "evlist.h"
#include "evsel.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "pmu.h"
#include "pmu-hybrid.h"
#include "print-events.h"
diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
index ac3227ba769c..c82e7bc7c5ea 100644
--- a/tools/perf/util/pfm.c
+++ b/tools/perf/util/pfm.c
@@ -12,7 +12,7 @@
#include "util/parse-events.h"
#include "util/pmu.h"
#include "util/pfm.h"
-#include "util/strbuf.h"
+#include <api/strbuf.h>
#include <string.h>
#include <linux/kernel.h>
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 2bdeb89352e7..4648ccf0b50a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -27,7 +27,7 @@
#include "print-events.h"
#include "header.h"
#include "string2.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "fncache.h"
#include "pmu-hybrid.h"
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 0c24bc7afbca..e609970e2113 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -38,7 +38,7 @@
#include "probe-file.h"
#include "session.h"
#include "string2.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include <subcmd/pager.h>
#include <linux/ctype.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 3d50de3217d5..c1f1ef3f48d4 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -20,7 +20,7 @@
#include "dso.h"
#include "color.h"
#include "symbol.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include <api/fs/tracing_path.h>
#include <api/fs/fs.h>
#include "probe-event.h"
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 54b49ce85c9f..4368a9dffc35 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -24,7 +24,7 @@
#include "dso.h"
#include "debug.h"
#include "intlist.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "strlist.h"
#include "symbol.h"
#include "probe-finder.h"
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index e188f74698dd..32f00a340c0d 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -21,7 +21,7 @@
#include "evlist.h"
#include "srcline.h"
#include "strlist.h"
-#include "strbuf.h"
+#include <api/strbuf.h>
#include "mem-events.h"
#include "annotate.h"
#include "event.h"
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 3/7] tools lib subcmd: Add run_command_strbuf
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
2023-01-10 22:19 ` [PATCH v1 1/7] perf llvm: Fix inadvertent file creation Ian Rogers
2023-01-10 22:19 ` [PATCH v1 2/7] tools lib: Move strbuf to libapi Ian Rogers
@ 2023-01-10 22:19 ` Ian Rogers
2023-01-10 22:20 ` [PATCH v1 4/7] tools lib api: Minor strbuf_read improvements Ian Rogers
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:19 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
Often a command wants to be run in a shell with stdout placed in a
string. This API mimics that of stdio's popen, running the given
command (not argv array) with "/bin/sh -c" then appending the output
from stdout to the buf argument.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/lib/subcmd/Makefile | 32 +++++++++++++++++++++++++++++---
tools/lib/subcmd/run-command.c | 30 ++++++++++++++++++++++++++++++
tools/lib/subcmd/run-command.h | 14 ++++++++++++++
3 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index b87213263a5e..23174d013519 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -13,6 +13,7 @@ CC ?= $(CROSS_COMPILE)gcc
LD ?= $(CROSS_COMPILE)ld
AR ?= $(CROSS_COMPILE)ar
+MKDIR = mkdir
RM = rm -f
MAKEFLAGS += --no-print-directory
@@ -55,6 +56,17 @@ CFLAGS += -I$(srctree)/tools/include/
CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
+LIBAPI_DIR = $(srctree)/tools/lib/api/
+ifneq ($(OUTPUT),)
+ LIBAPI_OUTPUT = $(abspath $(OUTPUT))/libapi
+else
+ LIBAPI_OUTPUT = $(CURDIR)/libapi
+endif
+LIBAPI_DESTDIR = $(LIBAPI_OUTPUT)
+LIBAPI_INCLUDE = $(LIBAPI_DESTDIR)/include
+LIBAPI = $(LIBAPI_OUTPUT)/libapi.a
+CFLAGS += -I$(LIBAPI_OUTPUT)/include
+
SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
ifeq ($(LP64), 1)
@@ -76,7 +88,9 @@ include $(srctree)/tools/build/Makefile.include
all: fixdep $(LIBFILE)
-$(SUBCMD_IN): FORCE
+prepare: $(LIBAPI)
+
+$(SUBCMD_IN): FORCE prepare
@$(MAKE) $(build)=libsubcmd
$(LIBFILE): $(SUBCMD_IN)
@@ -113,10 +127,22 @@ install_headers: $(INSTALL_HDRS)
install: install_lib install_headers
-clean:
+$(LIBAPI_OUTPUT):
+ $(Q)$(MKDIR) -p $@
+
+$(LIBAPI): FORCE | $(LIBAPI_OUTPUT)
+ $(Q)$(MAKE) -C $(LIBAPI_DIR) O=$(LIBAPI_OUTPUT) \
+ DESTDIR=$(LIBAPI_DESTDIR) prefix= \
+ $@ install_headers
+
+$(LIBAPI)-clean:
+ $(call QUIET_CLEAN, libapi)
+ $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
+
+clean: $(LIBAPI)-clean
$(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
FORCE:
-.PHONY: clean FORCE
+.PHONY: clean FORCE prepare
diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c
index 5cdac2162532..e90b285b6720 100644
--- a/tools/lib/subcmd/run-command.c
+++ b/tools/lib/subcmd/run-command.c
@@ -7,6 +7,7 @@
#include <linux/string.h>
#include <errno.h>
#include <sys/wait.h>
+#include <api/strbuf.h>
#include "subcmd-util.h"
#include "run-command.h"
#include "exec-cmd.h"
@@ -227,3 +228,32 @@ int run_command_v_opt(const char **argv, int opt)
prepare_run_command_v_opt(&cmd, argv, opt);
return run_command(&cmd);
}
+
+int run_command_strbuf(const char *cmd, struct strbuf *buf)
+{
+ const char *argv[4] = {
+ "/bin/sh",
+ "-c",
+ cmd,
+ NULL
+ };
+ struct child_process child = {
+ .argv = argv,
+ .out = -1,
+ };
+ int err;
+ ssize_t read_sz;
+
+ err = start_command(&child);
+ if (err)
+ return err;
+
+ read_sz = strbuf_read(buf, child.out, 0);
+
+ err = finish_command(&child);
+ close(child.out);
+ if (read_sz < 0)
+ return (int)read_sz;
+
+ return err;
+}
diff --git a/tools/lib/subcmd/run-command.h b/tools/lib/subcmd/run-command.h
index 17d969c6add3..1f7a2af9248c 100644
--- a/tools/lib/subcmd/run-command.h
+++ b/tools/lib/subcmd/run-command.h
@@ -58,4 +58,18 @@ int run_command(struct child_process *);
#define RUN_COMMAND_STDOUT_TO_STDERR 4
int run_command_v_opt(const char **argv, int opt);
+struct strbuf;
+/**
+ * run_command_strbuf() - Run cmd using /bin/sh and place stdout in strbuf.
+ * @cmd: The command to run by "/bin/sh -c".
+ * @buf: The strbuf appended to by reading stdout.
+ *
+ * Similar to popen with fread, run the given command reading the stdout output
+ * to buf. As with popen, stderr output goes to the current processes stderr but
+ * may be redirected in cmd by using "2>&1".
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int run_command_strbuf(const char *cmd, struct strbuf *buf);
+
#endif /* __SUBCMD_RUN_COMMAND_H */
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 4/7] tools lib api: Minor strbuf_read improvements
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
` (2 preceding siblings ...)
2023-01-10 22:19 ` [PATCH v1 3/7] tools lib subcmd: Add run_command_strbuf Ian Rogers
@ 2023-01-10 22:20 ` Ian Rogers
2023-01-10 22:20 ` [PATCH v1 5/7] tools lib api: Tweak strbuf allocation size computation Ian Rogers
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:20 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
If a read is smaller than the remaining space, don't grow the buffer
as it is likely we've reached the end of the file. Make the grow
amounts a single page rather than just over 2 once the null terminator
is included.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/lib/api/strbuf.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/lib/api/strbuf.c b/tools/lib/api/strbuf.c
index 4639b2d02e62..eafa2c01f46a 100644
--- a/tools/lib/api/strbuf.c
+++ b/tools/lib/api/strbuf.c
@@ -143,25 +143,28 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, ssize_t hint)
size_t oldalloc = sb->alloc;
int ret;
- ret = strbuf_grow(sb, hint ? hint : 8192);
+ ret = strbuf_grow(sb, hint ? hint : 4095);
if (ret)
return ret;
for (;;) {
- ssize_t cnt;
+ ssize_t read_size;
+ size_t sb_remaining = sb->alloc - sb->len - 1;
- cnt = read(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
- if (cnt < 0) {
+ read_size = read(fd, sb->buf + sb->len, sb_remaining);
+ if (read_size < 0) {
if (oldalloc == 0)
strbuf_release(sb);
else
strbuf_setlen(sb, oldlen);
- return cnt;
+ return read_size;
}
- if (!cnt)
+ if (read_size == 0)
break;
- sb->len += cnt;
- ret = strbuf_grow(sb, 8192);
+ sb->len += read_size;
+ if ((size_t)read_size < sb_remaining)
+ continue;
+ ret = strbuf_grow(sb, 4095);
if (ret)
return ret;
}
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 5/7] tools lib api: Tweak strbuf allocation size computation
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
` (3 preceding siblings ...)
2023-01-10 22:20 ` [PATCH v1 4/7] tools lib api: Minor strbuf_read improvements Ian Rogers
@ 2023-01-10 22:20 ` Ian Rogers
2023-01-10 22:20 ` [PATCH v1 6/7] perf help: Use run_command_strbuf Ian Rogers
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:20 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
alloc_nr gives an estimate of the actual memory behind an allocation
but isn't accurate. Use malloc_usable_size to accurately set the
strbuf alloc, which potentially avoids realloc calls.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/lib/api/strbuf.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/lib/api/strbuf.c b/tools/lib/api/strbuf.c
index eafa2c01f46a..a3d7f96d8b9f 100644
--- a/tools/lib/api/strbuf.c
+++ b/tools/lib/api/strbuf.c
@@ -4,6 +4,7 @@
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/zalloc.h>
+#include <malloc.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
@@ -42,7 +43,6 @@ char *strbuf_detach(struct strbuf *sb, size_t *sz)
return res;
}
-#define alloc_nr(x) (((x)+16)*3/2)
int strbuf_grow(struct strbuf *sb, size_t extra)
{
char *buf;
@@ -54,9 +54,6 @@ int strbuf_grow(struct strbuf *sb, size_t extra)
if (nr <= sb->len)
return -E2BIG;
- if (alloc_nr(sb->alloc) > nr)
- nr = alloc_nr(sb->alloc);
-
/*
* Note that sb->buf == strbuf_slopbuf if sb->alloc == 0, and it is
* a static variable. Thus we have to avoid passing it to realloc.
@@ -66,10 +63,9 @@ int strbuf_grow(struct strbuf *sb, size_t extra)
return -ENOMEM;
sb->buf = buf;
- sb->alloc = nr;
+ sb->alloc = malloc_usable_size(buf);
return 0;
}
-#undef alloc_nr
int strbuf_addch(struct strbuf *sb, int c)
{
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 6/7] perf help: Use run_command_strbuf
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
` (4 preceding siblings ...)
2023-01-10 22:20 ` [PATCH v1 5/7] tools lib api: Tweak strbuf allocation size computation Ian Rogers
@ 2023-01-10 22:20 ` Ian Rogers
2023-01-10 22:20 ` [PATCH v1 7/7] perf llvm: Remove read_from_pipe Ian Rogers
2023-01-19 16:05 ` [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:20 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
Remove boiler plate by using library routine.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-help.c | 47 ++++++++++++---------------------------
1 file changed, 14 insertions(+), 33 deletions(-)
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 8874e1e0335b..1cb87358cd20 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -70,46 +70,27 @@ static const char *get_man_viewer_info(const char *name)
static int check_emacsclient_version(void)
{
struct strbuf buffer = STRBUF_INIT;
- struct child_process ec_process;
- const char *argv_ec[] = { "emacsclient", "--version", NULL };
- int version;
int ret = -1;
- /* emacsclient prints its version number on stderr */
- memset(&ec_process, 0, sizeof(ec_process));
- ec_process.argv = argv_ec;
- ec_process.err = -1;
- ec_process.stdout_to_stderr = 1;
- if (start_command(&ec_process)) {
- fprintf(stderr, "Failed to start emacsclient.\n");
- return -1;
- }
- if (strbuf_read(&buffer, ec_process.err, 20) < 0) {
- fprintf(stderr, "Failed to read emacsclient version\n");
- goto out;
- }
- close(ec_process.err);
-
/*
- * Don't bother checking return value, because "emacsclient --version"
- * seems to always exits with code 1.
+ * emacsclient may print its version number on stderr. Don't bother
+ * checking return value, because some "emacsclient --version" commands
+ * seem to always exits with code 1.
*/
- finish_command(&ec_process);
+ run_command_strbuf("emacsclient --version 2>&1", &buffer);
- if (!strstarts(buffer.buf, "emacsclient")) {
+ if (!strstarts(buffer.buf, "emacsclient"))
fprintf(stderr, "Failed to parse emacsclient version.\n");
- goto out;
- }
-
- version = atoi(buffer.buf + strlen("emacsclient"));
+ else {
+ int version = atoi(buffer.buf + strlen("emacsclient"));
- if (version < 22) {
- fprintf(stderr,
- "emacsclient version '%d' too old (< 22).\n",
- version);
- } else
- ret = 0;
-out:
+ if (version < 22) {
+ fprintf(stderr,
+ "emacsclient version '%d' too old (< 22).\n",
+ version);
+ } else
+ ret = 0;
+ }
strbuf_release(&buffer);
return ret;
}
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v1 7/7] perf llvm: Remove read_from_pipe
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
` (5 preceding siblings ...)
2023-01-10 22:20 ` [PATCH v1 6/7] perf help: Use run_command_strbuf Ian Rogers
@ 2023-01-10 22:20 ` Ian Rogers
2023-01-19 16:05 ` [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
7 siblings, 0 replies; 10+ messages in thread
From: Ian Rogers @ 2023-01-10 22:20 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
Cc: Ian Rogers
Switch to the common run_command_strbuf utility.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/bpf.c | 12 +--
tools/perf/tests/llvm.c | 18 ++--
tools/perf/tests/llvm.h | 3 +-
tools/perf/util/bpf-loader.c | 9 +-
tools/perf/util/llvm-utils.c | 184 +++++++----------------------------
tools/perf/util/llvm-utils.h | 6 +-
6 files changed, 61 insertions(+), 171 deletions(-)
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 17c023823713..b4a6afb41e40 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -13,6 +13,7 @@
#include <linux/filter.h>
#include <linux/kernel.h>
#include <linux/string.h>
+#include <api/strbuf.h>
#include <api/fs/fs.h>
#include <perf/mmap.h>
#include "tests.h"
@@ -216,14 +217,13 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
static int __test__bpf(int idx)
{
int ret;
- void *obj_buf;
- size_t obj_buf_sz;
+ struct strbuf obj_buf = STRBUF_INIT;
struct bpf_object *obj;
- ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
+ ret = test_llvm__fetch_bpf_obj(&obj_buf,
bpf_testcase_table[idx].prog_id,
false, NULL);
- if (ret != TEST_OK || !obj_buf || !obj_buf_sz) {
+ if (ret != TEST_OK || !obj_buf.len) {
pr_debug("Unable to get BPF object, %s\n",
bpf_testcase_table[idx].msg_compile_fail);
if ((idx == 0) || (ret == TEST_SKIP))
@@ -232,7 +232,7 @@ static int __test__bpf(int idx)
return TEST_FAIL;
}
- obj = prepare_bpf(obj_buf, obj_buf_sz,
+ obj = prepare_bpf(obj_buf.buf, obj_buf.len,
bpf_testcase_table[idx].name);
if ((!!bpf_testcase_table[idx].target_func) != (!!obj)) {
if (!obj)
@@ -274,7 +274,7 @@ static int __test__bpf(int idx)
}
out:
- free(obj_buf);
+ strbuf_release(&obj_buf);
bpf__clear();
return ret;
}
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 0bc25a56cfef..97090850b7f9 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -4,6 +4,7 @@
#include <string.h>
#include "tests.h"
#include "debug.h"
+#include <api/strbuf.h>
#ifdef HAVE_LIBBPF_SUPPORT
#include <bpf/libbpf.h>
@@ -45,8 +46,7 @@ static struct {
};
int
-test_llvm__fetch_bpf_obj(void **p_obj_buf,
- size_t *p_obj_buf_sz,
+test_llvm__fetch_bpf_obj(struct strbuf *obj_buf,
enum test_llvm__testcase idx,
bool force,
bool *should_load_fail)
@@ -83,9 +83,6 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf,
if (verbose == 0)
verbose = -1;
- *p_obj_buf = NULL;
- *p_obj_buf_sz = 0;
-
if (!llvm_param.clang_bpf_cmd_template)
goto out;
@@ -106,7 +103,7 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf,
clang_opt_old = llvm_param.clang_opt;
llvm_param.clang_opt = clang_opt_new;
- err = llvm__compile_bpf("-", p_obj_buf, p_obj_buf_sz);
+ err = llvm__compile_bpf("-", obj_buf);
llvm_param.clang_bpf_cmd_template = tmpl_old;
llvm_param.clang_opt = clang_opt_old;
@@ -127,24 +124,23 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf,
static int test__llvm(int subtest)
{
int ret;
- void *obj_buf = NULL;
- size_t obj_buf_sz = 0;
+ struct strbuf obj_buf = STRBUF_INIT;
bool should_load_fail = false;
if ((subtest < 0) || (subtest >= __LLVM_TESTCASE_MAX))
return TEST_FAIL;
- ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
+ ret = test_llvm__fetch_bpf_obj(&obj_buf,
subtest, false, &should_load_fail);
if (ret == TEST_OK && !should_load_fail) {
- ret = test__bpf_parsing(obj_buf, obj_buf_sz);
+ ret = test__bpf_parsing(obj_buf.buf, obj_buf.len);
if (ret != TEST_OK) {
pr_debug("Failed to parse test case '%s'\n",
bpf_source_table[subtest].desc);
}
}
- free(obj_buf);
+ strbuf_release(&obj_buf);
return ret;
}
diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h
index f68b0d9b8ae2..2294b66dd0b6 100644
--- a/tools/perf/tests/llvm.h
+++ b/tools/perf/tests/llvm.h
@@ -22,7 +22,8 @@ enum test_llvm__testcase {
__LLVM_TESTCASE_MAX,
};
-int test_llvm__fetch_bpf_obj(void **p_obj_buf, size_t *p_obj_buf_sz,
+struct strbuf;
+int test_llvm__fetch_bpf_obj(struct strbuf *obj_buf,
enum test_llvm__testcase index, bool force,
bool *should_load_fail);
#ifdef __cplusplus
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 6e9b06cf06ee..f3a5cf490141 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -16,6 +16,7 @@
#include <linux/zalloc.h>
#include <errno.h>
#include <stdlib.h>
+#include <api/strbuf.h>
#include "debug.h"
#include "evlist.h"
#include "bpf-loader.h"
@@ -245,10 +246,14 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
err = perf_clang__compile_bpf(filename, &obj_buf, &obj_buf_sz);
perf_clang__cleanup();
if (err) {
+ struct strbuf str_obj_buf = STRBUF_INIT;
+
pr_debug("bpf: builtin compilation failed: %d, try external compiler\n", err);
- err = llvm__compile_bpf(filename, &obj_buf, &obj_buf_sz);
- if (err)
+ err = llvm__compile_bpf(filename, &str_obj_buf);
+ if (err < 0)
return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE);
+ obj_buf = str_obj_buf.buf;
+ obj_buf_sz = str_obj_buf.len;
} else
pr_debug("bpf: successful builtin compilation\n");
obj = bpf_object__open_mem(obj_buf, obj_buf_sz, &opts);
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 4e8e243a6e4b..8a12160320f3 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -17,7 +17,9 @@
#include "config.h"
#include "util.h"
#include <sys/wait.h>
+#include <api/strbuf.h>
#include <subcmd/exec-cmd.h>
+#include <subcmd/run-command.h>
#define CLANG_BPF_CMD_DEFAULT_TEMPLATE \
"$CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS "\
@@ -125,92 +127,6 @@ static int search_program_and_warn(const char *def, const char *name,
return ret;
}
-#define READ_SIZE 4096
-static int
-read_from_pipe(const char *cmd, void **p_buf, size_t *p_read_sz)
-{
- int err = 0;
- void *buf = NULL;
- FILE *file = NULL;
- size_t read_sz = 0, buf_sz = 0;
- char serr[STRERR_BUFSIZE];
-
- file = popen(cmd, "r");
- if (!file) {
- pr_err("ERROR: unable to popen cmd: %s\n",
- str_error_r(errno, serr, sizeof(serr)));
- return -EINVAL;
- }
-
- while (!feof(file) && !ferror(file)) {
- /*
- * Make buf_sz always have obe byte extra space so we
- * can put '\0' there.
- */
- if (buf_sz - read_sz < READ_SIZE + 1) {
- void *new_buf;
-
- buf_sz = read_sz + READ_SIZE + 1;
- new_buf = realloc(buf, buf_sz);
-
- if (!new_buf) {
- pr_err("ERROR: failed to realloc memory\n");
- err = -ENOMEM;
- goto errout;
- }
-
- buf = new_buf;
- }
- read_sz += fread(buf + read_sz, 1, READ_SIZE, file);
- }
-
- if (buf_sz - read_sz < 1) {
- pr_err("ERROR: internal error\n");
- err = -EINVAL;
- goto errout;
- }
-
- if (ferror(file)) {
- pr_err("ERROR: error occurred when reading from pipe: %s\n",
- str_error_r(errno, serr, sizeof(serr)));
- err = -EIO;
- goto errout;
- }
-
- err = WEXITSTATUS(pclose(file));
- file = NULL;
- if (err) {
- err = -EINVAL;
- goto errout;
- }
-
- /*
- * If buf is string, give it terminal '\0' to make our life
- * easier. If buf is not string, that '\0' is out of space
- * indicated by read_sz so caller won't even notice it.
- */
- ((char *)buf)[read_sz] = '\0';
-
- if (!p_buf)
- free(buf);
- else
- *p_buf = buf;
-
- if (p_read_sz)
- *p_read_sz = read_sz;
- return 0;
-
-errout:
- if (file)
- pclose(file);
- free(buf);
- if (p_buf)
- *p_buf = NULL;
- if (p_read_sz)
- *p_read_sz = 0;
- return err;
-}
-
static inline void
force_set_env(const char *var, const char *value)
{
@@ -244,7 +160,7 @@ version_notice(void)
);
}
-static int detect_kbuild_dir(char **kbuild_dir)
+static int detect_kbuild_dir(struct strbuf *kbuild_dir)
{
const char *test_dir = llvm_param.kbuild_dir;
const char *prefix_dir = "";
@@ -276,10 +192,9 @@ static int detect_kbuild_dir(char **kbuild_dir)
if (access(autoconf_path, R_OK) == 0) {
free(autoconf_path);
- err = asprintf(kbuild_dir, "%s%s%s", prefix_dir, test_dir,
- suffix_dir);
+ err = (int)strbuf_addf(kbuild_dir, "%s%s%s", prefix_dir, test_dir, suffix_dir);
if (err < 0)
- return -ENOMEM;
+ return err;
return 0;
}
pr_debug("%s: Couldn't find \"%s\", missing kernel-devel package?.\n",
@@ -315,7 +230,7 @@ static const char *kinc_fetch_script =
"rm -rf $TMPDIR\n"
"exit $RET\n";
-void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts)
+void llvm__get_kbuild_opts(struct strbuf *kbuild_dir, struct strbuf *kbuild_include_opts)
{
static char *saved_kbuild_dir;
static char *saved_kbuild_include_opts;
@@ -324,22 +239,14 @@ void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts)
if (!kbuild_dir || !kbuild_include_opts)
return;
- *kbuild_dir = NULL;
- *kbuild_include_opts = NULL;
-
if (saved_kbuild_dir && saved_kbuild_include_opts &&
!IS_ERR(saved_kbuild_dir) && !IS_ERR(saved_kbuild_include_opts)) {
- *kbuild_dir = strdup(saved_kbuild_dir);
- *kbuild_include_opts = strdup(saved_kbuild_include_opts);
+ strbuf_addstr(kbuild_dir, saved_kbuild_dir);
+ strbuf_addstr(kbuild_include_opts, saved_kbuild_include_opts);
- if (*kbuild_dir && *kbuild_include_opts)
- return;
-
- zfree(kbuild_dir);
- zfree(kbuild_include_opts);
/*
- * Don't fall through: it may breaks saved_kbuild_dir and
- * saved_kbuild_include_opts if detect them again when
+ * Don't fallthrough: it may break the saved_kbuild_dir and
+ * saved_kbuild_include_opts if they are detected again when
* memory is low.
*/
return;
@@ -361,28 +268,26 @@ void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts)
goto errout;
}
- pr_debug("Kernel build dir is set to %s\n", *kbuild_dir);
- force_set_env("KBUILD_DIR", *kbuild_dir);
+ pr_debug("Kernel build dir is set to %s\n", kbuild_dir->buf);
+ force_set_env("KBUILD_DIR", kbuild_dir->buf);
force_set_env("KBUILD_OPTS", llvm_param.kbuild_opts);
- err = read_from_pipe(kinc_fetch_script,
- (void **)kbuild_include_opts,
- NULL);
+ err = run_command_strbuf(kinc_fetch_script, kbuild_include_opts);
if (err) {
pr_warning(
"WARNING:\tunable to get kernel include directories from '%s'\n"
"Hint:\tTry set clang include options using 'clang-bpf-cmd-template'\n"
" \toption in [llvm] section of ~/.perfconfig and set 'kbuild-dir'\n"
" \toption in [llvm] to \"\" to suppress this detection.\n\n",
- *kbuild_dir);
+ kbuild_dir->buf);
zfree(kbuild_dir);
goto errout;
}
- pr_debug("include option is set to %s\n", *kbuild_include_opts);
+ pr_debug("include option is set to %s\n", kbuild_include_opts->buf);
- saved_kbuild_dir = strdup(*kbuild_dir);
- saved_kbuild_include_opts = strdup(*kbuild_include_opts);
+ saved_kbuild_dir = strdup(kbuild_dir->buf);
+ saved_kbuild_include_opts = strdup(kbuild_include_opts->buf);
if (!saved_kbuild_dir || !saved_kbuild_include_opts) {
zfree(&saved_kbuild_dir);
@@ -446,24 +351,23 @@ void llvm__dump_obj(const char *path, void *obj_buf, size_t size)
free(obj_path);
}
-int llvm__compile_bpf(const char *path, void **p_obj_buf,
- size_t *p_obj_buf_sz)
+int llvm__compile_bpf(const char *path, struct strbuf *obj_buf)
{
- size_t obj_buf_sz;
- void *obj_buf = NULL;
int err, nr_cpus_avail;
unsigned int kernel_version;
char linux_version_code_str[64];
const char *clang_opt = llvm_param.clang_opt;
char clang_path[PATH_MAX], llc_path[PATH_MAX], abspath[PATH_MAX], nr_cpus_avail_str[64];
char serr[STRERR_BUFSIZE];
- char *kbuild_dir = NULL, *kbuild_include_opts = NULL,
- *perf_bpf_include_opts = NULL;
+ char *perf_bpf_include_opts = NULL;
const char *template = llvm_param.clang_bpf_cmd_template;
char *pipe_template = NULL;
const char *opts = llvm_param.opts;
- char *command_echo = NULL, *command_out;
+ char *command_echo = NULL;
char *libbpf_include_dir = system_path(LIBBPF_INCLUDE_DIR);
+ struct strbuf kbuild_dir = STRBUF_INIT;
+ struct strbuf kbuild_include_opts = STRBUF_INIT;
+ struct strbuf command_out = STRBUF_INIT;
if (path[0] != '-' && realpath(path, abspath) == NULL) {
err = errno;
@@ -501,9 +405,9 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
force_set_env("LINUX_VERSION_CODE", linux_version_code_str);
force_set_env("CLANG_EXEC", clang_path);
force_set_env("CLANG_OPTIONS", clang_opt);
- force_set_env("KERNEL_INC_OPTIONS", kbuild_include_opts);
+ force_set_env("KERNEL_INC_OPTIONS", kbuild_include_opts.buf);
force_set_env("PERF_BPF_INC_OPTIONS", perf_bpf_include_opts);
- force_set_env("WORKING_DIR", kbuild_dir ? : ".");
+ force_set_env("WORKING_DIR", kbuild_dir.len ? kbuild_dir.buf : ".");
if (opts) {
err = search_program_and_warn(llvm_param.llc_path, "llc", llc_path);
@@ -549,11 +453,11 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
SWAP_CHAR('&', '\006');
SWAP_CHAR('\a', '"');
}
- err = read_from_pipe(command_echo, (void **) &command_out, NULL);
- if (err)
+ err = run_command_strbuf(command_echo, &command_out);
+ if (err < 0)
goto errout;
- for (char *p = command_out; *p; p++) {
+ for (char *p = command_out.buf; *p; p++) {
SWAP_CHAR('\001', '<');
SWAP_CHAR('\002', '>');
SWAP_CHAR('\003', '"');
@@ -562,10 +466,10 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
SWAP_CHAR('\006', '&');
}
#undef SWAP_CHAR
- pr_debug("llvm compiling command : %s\n", command_out);
+ pr_debug("llvm compiling command : %s\n", command_out.buf);
- err = read_from_pipe(template, &obj_buf, &obj_buf_sz);
- if (err) {
+ err = run_command_strbuf(template, obj_buf);
+ if (err < 0) {
pr_err("ERROR:\tunable to compile %s\n", path);
pr_err("Hint:\tCheck error message shown above.\n");
pr_err("Hint:\tYou can also pre-compile it into .o using:\n");
@@ -574,33 +478,15 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf,
goto errout;
}
- free(command_echo);
- free(command_out);
- free(kbuild_dir);
- free(kbuild_include_opts);
- free(perf_bpf_include_opts);
- free(libbpf_include_dir);
-
- if (!p_obj_buf)
- free(obj_buf);
- else
- *p_obj_buf = obj_buf;
-
- if (p_obj_buf_sz)
- *p_obj_buf_sz = obj_buf_sz;
- return 0;
+ err = 0;
errout:
free(command_echo);
- free(kbuild_dir);
- free(kbuild_include_opts);
- free(obj_buf);
+ strbuf_release(&command_out);
+ strbuf_release(&kbuild_dir);
+ strbuf_release(&kbuild_include_opts);
free(perf_bpf_include_opts);
free(libbpf_include_dir);
free(pipe_template);
- if (p_obj_buf)
- *p_obj_buf = NULL;
- if (p_obj_buf_sz)
- *p_obj_buf_sz = 0;
return err;
}
diff --git a/tools/perf/util/llvm-utils.h b/tools/perf/util/llvm-utils.h
index 7878a0e3fa98..0ad25e42aa6e 100644
--- a/tools/perf/util/llvm-utils.h
+++ b/tools/perf/util/llvm-utils.h
@@ -56,13 +56,15 @@ struct llvm_param {
extern struct llvm_param llvm_param;
int perf_llvm_config(const char *var, const char *value);
-int llvm__compile_bpf(const char *path, void **p_obj_buf, size_t *p_obj_buf_sz);
+struct strbuf;
+int llvm__compile_bpf(const char *path, struct strbuf *obj_buf);
/* This function is for test__llvm() use only */
int llvm__search_clang(void);
/* Following functions are reused by builtin clang support */
-void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts);
+struct strbuf;
+void llvm__get_kbuild_opts(struct strbuf *kbuild_dir, struct strbuf *kbuild_include_opts);
int llvm__get_nr_cpus(void);
void llvm__dump_obj(const char *path, void *obj_buf, size_t size);
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 0/7] Add and use run_command_strbuf
2023-01-10 22:19 [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
` (6 preceding siblings ...)
2023-01-10 22:20 ` [PATCH v1 7/7] perf llvm: Remove read_from_pipe Ian Rogers
@ 2023-01-19 16:05 ` Ian Rogers
2023-01-19 16:28 ` Arnaldo Carvalho de Melo
7 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2023-01-19 16:05 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Nathan Chancellor, Nick Desaulniers, Tom Rix, Nicolas Schier,
Masahiro Yamada, Athira Rajeev, Christy Lee, Andrii Nakryiko,
Ravi Bangoria, Leo Yan, Yang Jihong, Qi Liu, James Clark,
Adrian Hunter, Masami Hiramatsu (Google), Kan Liang,
Sean Christopherson, Zhengjun Xing, Rob Herring, Xin Gao,
Zechuan Chen, Jason Wang, Christophe JAILLET, Stephane Eranian,
German Gomez, linux-kernel, linux-perf-users, bpf, llvm
On Tue, Jan 10, 2023 at 2:20 PM Ian Rogers <irogers@google.com> wrote:
>
> It is commonly useful to run a command using "/bin/sh -c" (like popen)
> and to place the output in a string. Move strbuf to libapi, add a new
> run_command that places output in a strbuf, then use it in help and
> llvm in perf. Some small strbuf efficiency improvements are
> included. Whilst adding a new function should increase lines-of-code,
> by sharing two similar usages in perf llvm and perf help, the overall
> lines-of-code is moderately reduced.
>
> First "perf llvm: Fix inadvertent file creation" is cherry-picked
> from:
> https://lore.kernel.org/lkml/20230105082609.344538-1-irogers@google.com/
> to avoid a merge conflict. The next patches deal with moving strbuf,
> adding the run_command function with Makefile dependency from
> libsubcmd to libapi, and improving the strbuf performance. The final
> two patches add usage from the perf command.
>
> Ian Rogers (7):
> perf llvm: Fix inadvertent file creation
> tools lib: Move strbuf to libapi
> tools lib subcmd: Add run_command_strbuf
> tools lib api: Minor strbuf_read improvements
> tools lib api: Tweak strbuf allocation size computation
> perf help: Use run_command_strbuf
> perf llvm: Remove read_from_pipe
This isn't ready yet. Kernel test robot reported legitimate build
breakages in other tools outside of perf, I'm looking to address those
in separate patch series.
https://lore.kernel.org/lkml/20230116215751.633675-1-irogers@google.com/
Thanks,
Ian
> tools/lib/api/Build | 1 +
> tools/lib/api/Makefile | 2 +-
> tools/{perf/util => lib/api}/strbuf.c | 28 ++--
> tools/{perf/util => lib/api}/strbuf.h | 0
> tools/lib/subcmd/Makefile | 32 +++-
> tools/lib/subcmd/run-command.c | 30 ++++
> tools/lib/subcmd/run-command.h | 14 ++
> tools/perf/bench/evlist-open-close.c | 2 +-
> tools/perf/builtin-help.c | 49 ++----
> tools/perf/builtin-list.c | 2 +-
> tools/perf/tests/bpf.c | 12 +-
> tools/perf/tests/llvm.c | 18 +--
> tools/perf/tests/llvm.h | 3 +-
> tools/perf/util/Build | 1 -
> tools/perf/util/bpf-loader.c | 9 +-
> tools/perf/util/cache.h | 2 +-
> tools/perf/util/dwarf-aux.c | 2 +-
> tools/perf/util/env.c | 2 +-
> tools/perf/util/header.c | 2 +-
> tools/perf/util/llvm-utils.c | 207 ++++++++------------------
> tools/perf/util/llvm-utils.h | 6 +-
> tools/perf/util/metricgroup.c | 2 +-
> tools/perf/util/pfm.c | 2 +-
> tools/perf/util/pmu.c | 2 +-
> tools/perf/util/probe-event.c | 2 +-
> tools/perf/util/probe-file.c | 2 +-
> tools/perf/util/probe-finder.c | 2 +-
> tools/perf/util/sort.c | 2 +-
> 28 files changed, 201 insertions(+), 237 deletions(-)
> rename tools/{perf/util => lib/api}/strbuf.c (87%)
> rename tools/{perf/util => lib/api}/strbuf.h (100%)
>
> --
> 2.39.0.314.g84b9a713c41-goog
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 0/7] Add and use run_command_strbuf
2023-01-19 16:05 ` [PATCH v1 0/7] Add and use run_command_strbuf Ian Rogers
@ 2023-01-19 16:28 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-01-19 16:28 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Nathan Chancellor, Nick Desaulniers,
Tom Rix, Nicolas Schier, Masahiro Yamada, Athira Rajeev,
Christy Lee, Andrii Nakryiko, Ravi Bangoria, Leo Yan, Yang Jihong,
Qi Liu, James Clark, Adrian Hunter, Masami Hiramatsu (Google),
Kan Liang, Sean Christopherson, Zhengjun Xing, Rob Herring,
Xin Gao, Zechuan Chen, Jason Wang, Christophe JAILLET,
Stephane Eranian, German Gomez, linux-kernel, linux-perf-users,
bpf, llvm
Em Thu, Jan 19, 2023 at 08:05:36AM -0800, Ian Rogers escreveu:
> On Tue, Jan 10, 2023 at 2:20 PM Ian Rogers <irogers@google.com> wrote:
> >
> > It is commonly useful to run a command using "/bin/sh -c" (like popen)
> > and to place the output in a string. Move strbuf to libapi, add a new
> > run_command that places output in a strbuf, then use it in help and
> > llvm in perf. Some small strbuf efficiency improvements are
> > included. Whilst adding a new function should increase lines-of-code,
> > by sharing two similar usages in perf llvm and perf help, the overall
> > lines-of-code is moderately reduced.
> >
> > First "perf llvm: Fix inadvertent file creation" is cherry-picked
> > from:
> > https://lore.kernel.org/lkml/20230105082609.344538-1-irogers@google.com/
> > to avoid a merge conflict. The next patches deal with moving strbuf,
> > adding the run_command function with Makefile dependency from
> > libsubcmd to libapi, and improving the strbuf performance. The final
> > two patches add usage from the perf command.
> >
> > Ian Rogers (7):
> > perf llvm: Fix inadvertent file creation
> > tools lib: Move strbuf to libapi
> > tools lib subcmd: Add run_command_strbuf
> > tools lib api: Minor strbuf_read improvements
> > tools lib api: Tweak strbuf allocation size computation
> > perf help: Use run_command_strbuf
> > perf llvm: Remove read_from_pipe
>
> This isn't ready yet. Kernel test robot reported legitimate build
> breakages in other tools outside of perf, I'm looking to address those
> in separate patch series.
> https://lore.kernel.org/lkml/20230116215751.633675-1-irogers@google.com/
Thanks for the heads up, I recall seeing a build bot report.
It's great to have that build bot testing perf patches, thanks to
whoever put it in place!
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread