* [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole
@ 2022-01-26 19:20 Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads Kui-Feng Lee
` (3 more replies)
0 siblings, 4 replies; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 19:20 UTC (permalink / raw)
To: dwarves, arnaldo.melo; +Cc: ast, daniel, andrii, bpf, Kui-Feng Lee
Creating an instance of btf for each worker thread allows
steal-function provided by pahole to add type info on multiple threads
without a lock. The main thread merges the results of worker threads
to the primary instance.
Copying data from per-thread btf instances to the primary instance is
expensive now. However, there is a patch landed at the bpf-next
repository. [1] With the patch for bpf-next and this patch, they drop
total runtime to 5.4s from 6.0s with "-j4" on my device to generate
BTF for Linux.
V4 includes following changes.
- Fix nits and typos.
- Rollback to calling btf__add_btf() at the main thread to simplify
code. The reasons for that are additional lock making it
complicated and doing w/o reusing btf_encoder being obvious slower.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=d81283d27266
[v1] https://lore.kernel.org/dwarves/20220120010817.2803482-1-kuifeng@fb.com/
[v2] https://lore.kernel.org/dwarves/20220124191858.1601255-1-kuifeng@fb.com/
[v3] https://lore.kernel.org/dwarves/20220126040509.1862767-1-kuifeng@fb.com/
Kui-Feng Lee (4):
dwarf_loader: Receive per-thread data on worker threads.
dwarf_loader: Prepare and pass per-thread data to worker threads.
pahole: Use per-thread btf instances to avoid mutex locking.
libbpf: Update libbpf to a new revision.
btf_encoder.c | 25 ++++++----
btf_encoder.h | 2 +
btf_loader.c | 4 +-
ctf_loader.c | 2 +-
dwarf_loader.c | 59 ++++++++++++++++++-----
dwarves.h | 9 +++-
lib/bpf | 2 +-
pahole.c | 128 +++++++++++++++++++++++++++++++++++++++++++++----
pdwtags.c | 3 +-
pfunct.c | 4 +-
10 files changed, 198 insertions(+), 40 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-01-26 19:20 [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole Kui-Feng Lee
@ 2022-01-26 19:20 ` Kui-Feng Lee
2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 19:20 ` [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to " Kui-Feng Lee
` (2 subsequent siblings)
3 siblings, 1 reply; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 19:20 UTC (permalink / raw)
To: dwarves, arnaldo.melo; +Cc: ast, daniel, andrii, bpf, Kui-Feng Lee
Add arguments to steal and thread_exit callbacks of conf_load to
receive per-thread data.
Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
---
btf_loader.c | 2 +-
ctf_loader.c | 2 +-
dwarf_loader.c | 4 ++--
dwarves.h | 5 +++--
pahole.c | 3 ++-
pdwtags.c | 3 ++-
pfunct.c | 4 +++-
7 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/btf_loader.c b/btf_loader.c
index 7a5b16ff393e..b61cadd55127 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
* The app stole this cu, possibly deleting it,
* so forget about it
*/
- if (conf && conf->steal && conf->steal(cu, conf))
+ if (conf && conf->steal && conf->steal(cu, conf, NULL))
return 0;
cus__add(cus, cu);
diff --git a/ctf_loader.c b/ctf_loader.c
index 7c34739afdce..de6d4dbfce97 100644
--- a/ctf_loader.c
+++ b/ctf_loader.c
@@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
* The app stole this cu, possibly deleting it,
* so forget about it
*/
- if (conf && conf->steal && conf->steal(cu, conf))
+ if (conf && conf->steal && conf->steal(cu, conf, NULL))
return 0;
cus__add(cus, cu);
diff --git a/dwarf_loader.c b/dwarf_loader.c
index e30b03c1c541..bf9ea3765419 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
{
cu__for_all_tags(cu, class_member__cache_byte_size, conf);
if (conf && conf->steal) {
- return conf->steal(cu, conf);
+ return conf->steal(cu, conf, NULL);
}
return LSK__KEEPIT;
}
@@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
goto out_abort;
}
- if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
+ if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
goto out_abort;
return (void *)DWARF_CB_OK;
diff --git a/dwarves.h b/dwarves.h
index 52d162d67456..9a8e4de8843a 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -48,8 +48,9 @@ struct conf_fprintf;
*/
struct conf_load {
enum load_steal_kind (*steal)(struct cu *cu,
- struct conf_load *conf);
- int (*thread_exit)(void);
+ struct conf_load *conf,
+ void *thr_data);
+ int (*thread_exit)(struct conf_load *conf, void *thr_data);
void *cookie;
char *format_path;
int nr_jobs;
diff --git a/pahole.c b/pahole.c
index f3a51cb2fe74..f3eeaaca4cdf 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2799,7 +2799,8 @@ out:
static struct type_instance *header;
static enum load_steal_kind pahole_stealer(struct cu *cu,
- struct conf_load *conf_load)
+ struct conf_load *conf_load,
+ void *thr_data)
{
int ret = LSK__DELETE;
diff --git a/pdwtags.c b/pdwtags.c
index 2b5ba1bf6745..8b1d6f1c96cb 100644
--- a/pdwtags.c
+++ b/pdwtags.c
@@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
}
static enum load_steal_kind pdwtags_stealer(struct cu *cu,
- struct conf_load *conf_load __maybe_unused)
+ struct conf_load *conf_load __maybe_unused,
+ void *thr_data __maybe_unused)
{
cu__emit_tags(cu);
return LSK__DELETE;
diff --git a/pfunct.c b/pfunct.c
index 5485622e639b..314915b774f4 100644
--- a/pfunct.c
+++ b/pfunct.c
@@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
return EXIT_SUCCESS;
}
-static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
+static enum load_steal_kind pfunct_stealer(struct cu *cu,
+ struct conf_load *conf_load __maybe_unused,
+ void *thr_data __maybe_unused)
{
if (function_name) {
--
2.30.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to worker threads.
2022-01-26 19:20 [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads Kui-Feng Lee
@ 2022-01-26 19:20 ` Kui-Feng Lee
2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 19:20 ` [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision Kui-Feng Lee
3 siblings, 1 reply; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 19:20 UTC (permalink / raw)
To: dwarves, arnaldo.melo; +Cc: ast, daniel, andrii, bpf, Kui-Feng Lee
Add interfaces to allow users of dwarf_loader to prepare and pass
per-thread data to steal-functions running on worker threads.
Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
---
dwarf_loader.c | 59 +++++++++++++++++++++++++++++++++++++++-----------
dwarves.h | 4 ++++
2 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/dwarf_loader.c b/dwarf_loader.c
index bf9ea3765419..151bc836c787 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2682,18 +2682,18 @@ static int class_member__cache_byte_size(struct tag *tag, struct cu *cu,
return 0;
}
-static int cu__finalize(struct cu *cu, struct conf_load *conf)
+static int cu__finalize(struct cu *cu, struct conf_load *conf, void *thr_data)
{
cu__for_all_tags(cu, class_member__cache_byte_size, conf);
if (conf && conf->steal) {
- return conf->steal(cu, conf, NULL);
+ return conf->steal(cu, conf, thr_data);
}
return LSK__KEEPIT;
}
-static int cus__finalize(struct cus *cus, struct cu *cu, struct conf_load *conf)
+static int cus__finalize(struct cus *cus, struct cu *cu, struct conf_load *conf, void *thr_data)
{
- int lsk = cu__finalize(cu, conf);
+ int lsk = cu__finalize(cu, conf, thr_data);
switch (lsk) {
case LSK__DELETE:
cu__delete(cu);
@@ -2862,7 +2862,13 @@ struct dwarf_cus {
struct dwarf_cu *type_dcu;
};
-static int dwarf_cus__create_and_process_cu(struct dwarf_cus *dcus, Dwarf_Die *cu_die, uint8_t pointer_size)
+struct dwarf_thread {
+ struct dwarf_cus *dcus;
+ void *data;
+};
+
+static int dwarf_cus__create_and_process_cu(struct dwarf_cus *dcus, Dwarf_Die *cu_die,
+ uint8_t pointer_size, void *thr_data)
{
/*
* DW_AT_name in DW_TAG_compile_unit can be NULL, first seen in:
@@ -2884,7 +2890,7 @@ static int dwarf_cus__create_and_process_cu(struct dwarf_cus *dcus, Dwarf_Die *c
cu->dfops = &dwarf__ops;
if (die__process_and_recode(cu_die, cu, dcus->conf) != 0 ||
- cus__finalize(dcus->cus, cu, dcus->conf) == LSK__STOP_LOADING)
+ cus__finalize(dcus->cus, cu, dcus->conf, thr_data) == LSK__STOP_LOADING)
return DWARF_CB_ABORT;
return DWARF_CB_OK;
@@ -2918,7 +2924,8 @@ out_unlock:
static void *dwarf_cus__process_cu_thread(void *arg)
{
- struct dwarf_cus *dcus = arg;
+ struct dwarf_thread *dthr = arg;
+ struct dwarf_cus *dcus = dthr->dcus;
uint8_t pointer_size, offset_size;
Dwarf_Die die_mem, *cu_die;
@@ -2926,11 +2933,13 @@ static void *dwarf_cus__process_cu_thread(void *arg)
if (cu_die == NULL)
break;
- if (dwarf_cus__create_and_process_cu(dcus, cu_die, pointer_size) == DWARF_CB_ABORT)
+ if (dwarf_cus__create_and_process_cu(dcus, cu_die,
+ pointer_size, dthr->data) == DWARF_CB_ABORT)
goto out_abort;
}
- if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
+ if (dcus->conf->thread_exit &&
+ dcus->conf->thread_exit(dcus->conf, dthr->data) != 0)
goto out_abort;
return (void *)DWARF_CB_OK;
@@ -2941,10 +2950,26 @@ out_abort:
static int dwarf_cus__threaded_process_cus(struct dwarf_cus *dcus)
{
pthread_t threads[dcus->conf->nr_jobs];
+ struct dwarf_thread dthr[dcus->conf->nr_jobs];
+ void *thread_data[dcus->conf->nr_jobs];
+ int res;
int i;
+ if (dcus->conf->threads_prepare) {
+ res = dcus->conf->threads_prepare(dcus->conf, dcus->conf->nr_jobs, thread_data);
+ if (res != 0)
+ return res;
+ } else {
+ memset(thread_data, 0, sizeof(void *) * dcus->conf->nr_jobs);
+ }
+
for (i = 0; i < dcus->conf->nr_jobs; ++i) {
- dcus->error = pthread_create(&threads[i], NULL, dwarf_cus__process_cu_thread, dcus);
+ dthr[i].dcus = dcus;
+ dthr[i].data = thread_data[i];
+
+ dcus->error = pthread_create(&threads[i], NULL,
+ dwarf_cus__process_cu_thread,
+ &dthr[i]);
if (dcus->error)
goto out_join;
}
@@ -2960,6 +2985,13 @@ out_join:
dcus->error = (long)res;
}
+ if (dcus->conf->threads_collect) {
+ res = dcus->conf->threads_collect(dcus->conf, dcus->conf->nr_jobs,
+ thread_data, dcus->error);
+ if (dcus->error == 0)
+ dcus->error = res;
+ }
+
return dcus->error;
}
@@ -2976,7 +3008,8 @@ static int __dwarf_cus__process_cus(struct dwarf_cus *dcus)
if (cu_die == NULL)
break;
- if (dwarf_cus__create_and_process_cu(dcus, cu_die, pointer_size) == DWARF_CB_ABORT)
+ if (dwarf_cus__create_and_process_cu(dcus, cu_die,
+ pointer_size, NULL) == DWARF_CB_ABORT)
return DWARF_CB_ABORT;
dcus->off = noff;
@@ -3070,7 +3103,7 @@ static int cus__merge_and_process_cu(struct cus *cus, struct conf_load *conf,
if (cu__resolve_func_ret_types(cu) != LSK__KEEPIT)
goto out_abort;
- if (cus__finalize(cus, cu, conf) == LSK__STOP_LOADING)
+ if (cus__finalize(cus, cu, conf, NULL) == LSK__STOP_LOADING)
goto out_abort;
return 0;
@@ -3102,7 +3135,7 @@ static int cus__load_module(struct cus *cus, struct conf_load *conf,
}
if (type_cu != NULL) {
- type_lsk = cu__finalize(type_cu, conf);
+ type_lsk = cu__finalize(type_cu, conf, NULL);
if (type_lsk == LSK__KEEPIT) {
cus__add(cus, type_cu);
}
diff --git a/dwarves.h b/dwarves.h
index 9a8e4de8843a..de152f9b64cf 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -71,6 +71,10 @@ struct conf_load {
const char *kabi_prefix;
struct btf *base_btf;
struct conf_fprintf *conf_fprintf;
+ int (*threads_prepare)(struct conf_load *conf, int nr_threads,
+ void **thr_data);
+ int (*threads_collect)(struct conf_load *conf, int nr_threads,
+ void **thr_data, int error);
};
/** struct conf_fprintf - hints to the __fprintf routines
--
2.30.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-26 19:20 [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to " Kui-Feng Lee
@ 2022-01-26 19:20 ` Kui-Feng Lee
2022-01-26 19:58 ` Andrii Nakryiko
2022-01-26 19:20 ` [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision Kui-Feng Lee
3 siblings, 1 reply; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 19:20 UTC (permalink / raw)
To: dwarves, arnaldo.melo; +Cc: ast, daniel, andrii, bpf, Kui-Feng Lee
Create an instance of btf for each worker thread, and add type info to
the local btf instance in the steal-function of pahole without mutex
acquiring. Once finished with all worker threads, merge all
per-thread btf instances to the primary btf instance.
Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
---
btf_encoder.c | 5 ++
btf_encoder.h | 2 +
pahole.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 124 insertions(+), 8 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index 9d015f304e92..56a76f5d7275 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1529,3 +1529,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
out:
return err;
}
+
+struct btf *btf_encoder__btf(struct btf_encoder *encoder)
+{
+ return encoder->btf;
+}
diff --git a/btf_encoder.h b/btf_encoder.h
index f133b0d7674d..0f0eee84df74 100644
--- a/btf_encoder.h
+++ b/btf_encoder.h
@@ -29,4 +29,6 @@ struct btf_encoder *btf_encoders__first(struct list_head *encoders);
struct btf_encoder *btf_encoders__next(struct btf_encoder *encoder);
+struct btf *btf_encoder__btf(struct btf_encoder *encoder);
+
#endif /* _BTF_ENCODER_H_ */
diff --git a/pahole.c b/pahole.c
index f3eeaaca4cdf..8dcd6bf951fe 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2798,6 +2798,72 @@ out:
static struct type_instance *header;
+struct thread_data {
+ struct btf *btf;
+ struct btf_encoder *encoder;
+};
+
+static int pahole_threads_prepare(struct conf_load *conf, int nr_threads, void **thr_data)
+{
+ int i;
+ struct thread_data *threads = calloc(sizeof(struct thread_data), nr_threads);
+
+ for (i = 0; i < nr_threads; i++)
+ thr_data[i] = threads + i;
+
+ return 0;
+}
+
+static int pahole_thread_exit(struct conf_load *conf, void *thr_data)
+{
+ struct thread_data *thread = thr_data;
+
+ if (thread == NULL)
+ return 0;
+
+ /*
+ * Here we will call btf__dedup() here once we extend
+ * btf__dedup().
+ */
+
+ return 0;
+}
+
+static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void **thr_data,
+ int error)
+{
+ struct thread_data **threads = (struct thread_data **)thr_data;
+ int i;
+ int err = 0;
+
+ if (error)
+ goto out;
+
+ for (i = 0; i < nr_threads; i++) {
+ /*
+ * Merge content of the btf instances of worker threads to the btf
+ * instance of the primary btf_encoder.
+ */
+ if (!threads[i]->btf || threads[i]->encoder == btf_encoder)
+ continue; /* The primary btf_encoder */
+ err = btf__add_btf(btf_encoder__btf(btf_encoder), threads[i]->btf);
+ if (err < 0)
+ goto out;
+ btf_encoder__delete(threads[i]->encoder);
+ threads[i]->encoder = NULL;
+ }
+ err = 0;
+
+out:
+ for (i = 0; i < nr_threads; i++) {
+ if (threads[i]->encoder && threads[i]->encoder != btf_encoder)
+ btf_encoder__delete(threads[i]->encoder);
+ }
+ free(threads[0]);
+
+ return err;
+}
+
static enum load_steal_kind pahole_stealer(struct cu *cu,
struct conf_load *conf_load,
void *thr_data)
@@ -2819,30 +2885,70 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
if (btf_encode) {
static pthread_mutex_t btf_lock = PTHREAD_MUTEX_INITIALIZER;
+ struct btf_encoder *encoder;
- pthread_mutex_lock(&btf_lock);
/*
* FIXME:
*
* This should be really done at main(), but since in the current codebase only at this
* point we'll have cu->elf setup...
*/
+ pthread_mutex_lock(&btf_lock);
if (!btf_encoder) {
- btf_encoder = btf_encoder__new(cu, detached_btf_filename, conf_load->base_btf, skip_encoding_btf_vars,
- btf_encode_force, btf_gen_floats, global_verbose);
- if (btf_encoder == NULL) {
- ret = LSK__STOP_LOADING;
- goto out_btf;
+ /*
+ * btf_encoder is the primary encoder.
+ * And, it is used by the thread
+ * create it.
+ */
+ btf_encoder = btf_encoder__new(cu, detached_btf_filename,
+ conf_load->base_btf,
+ skip_encoding_btf_vars,
+ btf_encode_force,
+ btf_gen_floats, global_verbose);
+ if (btf_encoder && thr_data) {
+ struct thread_data *thread = thr_data;
+
+ thread->encoder = btf_encoder;
+ thread->btf = btf_encoder__btf(btf_encoder);
}
}
+ pthread_mutex_unlock(&btf_lock);
+
+ if (btf_encoder == NULL) {
+ ret = LSK__STOP_LOADING;
+ goto out_btf;
+ }
- if (btf_encoder__encode_cu(btf_encoder, cu)) {
+ /*
+ * thr_data keeps per-thread data for worker threads. Each worker thread
+ * has an encoder. The main thread will merge the data collected by all
+ * these encoders to btf_encoder. However, the first thread reaching this
+ * function creates btf_encoder and reuses it as its local encoder. It
+ * avoids copying the data collected by the first thread.
+ */
+ if (thr_data) {
+ struct thread_data *thread = (struct thread_data *)thr_data;
+
+ if (thread->encoder == NULL) {
+ thread->encoder =
+ btf_encoder__new(cu, detached_btf_filename,
+ NULL,
+ skip_encoding_btf_vars,
+ btf_encode_force,
+ btf_gen_floats,
+ global_verbose);
+ thread->btf = btf_encoder__btf(thread->encoder);
+ }
+ encoder = thread->encoder;
+ } else
+ encoder = btf_encoder;
+
+ if (btf_encoder__encode_cu(encoder, cu)) {
fprintf(stderr, "Encountered error while encoding BTF.\n");
exit(1);
}
ret = LSK__DELETE;
out_btf:
- pthread_mutex_unlock(&btf_lock);
return ret;
}
#if 0
@@ -3207,6 +3313,9 @@ int main(int argc, char *argv[])
memset(tab, ' ', sizeof(tab) - 1);
conf_load.steal = pahole_stealer;
+ conf_load.thread_exit = pahole_thread_exit;
+ conf_load.threads_prepare = pahole_threads_prepare;
+ conf_load.threads_collect = pahole_threads_collect;
// Make 'pahole --header type < file' a shorter form of 'pahole -C type --count 1 < file'
if (conf.header_type && !class_name && prettify_input) {
--
2.30.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision.
2022-01-26 19:20 [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole Kui-Feng Lee
` (2 preceding siblings ...)
2022-01-26 19:20 ` [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking Kui-Feng Lee
@ 2022-01-26 19:20 ` Kui-Feng Lee
2022-01-26 19:59 ` Andrii Nakryiko
3 siblings, 1 reply; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 19:20 UTC (permalink / raw)
To: dwarves, arnaldo.melo; +Cc: ast, daniel, andrii, bpf, Kui-Feng Lee
Replace deprecated APIs with new ones.
Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
---
btf_encoder.c | 20 ++++++++++----------
btf_loader.c | 2 +-
lib/bpf | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index 56a76f5d7275..fa29824f11fb 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -172,7 +172,7 @@ __attribute ((format (printf, 5, 6)))
static void btf__log_err(const struct btf *btf, int kind, const char *name,
bool output_cr, const char *fmt, ...)
{
- fprintf(stderr, "[%u] %s %s", btf__get_nr_types(btf) + 1,
+ fprintf(stderr, "[%u] %s %s", btf__type_cnt(btf),
btf_kind_str[kind], name ?: "(anon)");
if (fmt && *fmt) {
@@ -203,7 +203,7 @@ static void btf_encoder__log_type(const struct btf_encoder *encoder, const struc
out = err ? stderr : stdout;
fprintf(out, "[%u] %s %s",
- btf__get_nr_types(btf), btf_kind_str[kind],
+ btf__type_cnt(btf) - 1, btf_kind_str[kind],
btf__printable_name(btf, t->name_off));
if (fmt && *fmt) {
@@ -449,10 +449,10 @@ static int btf_encoder__add_field(struct btf_encoder *encoder, const char *name,
int err;
err = btf__add_field(btf, name, type, offset, bitfield_size);
- t = btf__type_by_id(btf, btf__get_nr_types(btf));
+ t = btf__type_by_id(btf, btf__type_cnt(btf) - 1);
if (err) {
fprintf(stderr, "[%u] %s %s's field '%s' offset=%u bit_size=%u type=%u Error emitting field\n",
- btf__get_nr_types(btf), btf_kind_str[btf_kind(t)],
+ btf__type_cnt(btf) - 1, btf_kind_str[btf_kind(t)],
btf__printable_name(btf, t->name_off),
name, offset, bitfield_size, type);
} else {
@@ -899,9 +899,9 @@ static int btf_encoder__write_raw_file(struct btf_encoder *encoder)
const void *raw_btf_data;
int fd, err;
- raw_btf_data = btf__get_raw_data(encoder->btf, &raw_btf_size);
+ raw_btf_data = btf__raw_data(encoder->btf, &raw_btf_size);
if (raw_btf_data == NULL) {
- fprintf(stderr, "%s: btf__get_raw_data failed!\n", __func__);
+ fprintf(stderr, "%s: btf__raw_data failed!\n", __func__);
return -1;
}
@@ -976,7 +976,7 @@ static int btf_encoder__write_elf(struct btf_encoder *encoder)
}
}
- raw_btf_data = btf__get_raw_data(btf, &raw_btf_size);
+ raw_btf_data = btf__raw_data(btf, &raw_btf_size);
if (btf_data) {
/* Existing .BTF section found */
@@ -1043,10 +1043,10 @@ int btf_encoder__encode(struct btf_encoder *encoder)
btf_encoder__add_datasec(encoder, PERCPU_SECTION);
/* Empty file, nothing to do, so... done! */
- if (btf__get_nr_types(encoder->btf) == 0)
+ if (btf__type_cnt(encoder->btf) == 1)
return 0;
- if (btf__dedup(encoder->btf, NULL, NULL)) {
+ if (btf__dedup(encoder->btf, NULL)) {
fprintf(stderr, "%s: btf__dedup failed!\n", __func__);
return -1;
}
@@ -1403,7 +1403,7 @@ void btf_encoder__delete(struct btf_encoder *encoder)
int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu)
{
- uint32_t type_id_off = btf__get_nr_types(encoder->btf);
+ uint32_t type_id_off = btf__type_cnt(encoder->btf) - 1;
struct llvm_annotation *annot;
int btf_type_id, tag_type_id;
uint32_t core_id;
diff --git a/btf_loader.c b/btf_loader.c
index b61cadd55127..b5d444643adf 100644
--- a/btf_loader.c
+++ b/btf_loader.c
@@ -399,7 +399,7 @@ static int btf__load_types(struct btf *btf, struct cu *cu)
uint32_t type_index;
int err;
- for (type_index = 1; type_index <= btf__get_nr_types(btf); type_index++) {
+ for (type_index = 1; type_index < btf__type_cnt(btf); type_index++) {
const struct btf_type *type_ptr = btf__type_by_id(btf, type_index);
uint32_t type = btf_kind(type_ptr);
diff --git a/lib/bpf b/lib/bpf
index 94a49850c5ee..393a058d061d 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 94a49850c5ee61ea02dfcbabf48013391e8cecdf
+Subproject commit 393a058d061d49d5c3055fa9eefafb4c0c31ccc3
--
2.30.2
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-01-26 19:20 ` [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads Kui-Feng Lee
@ 2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 20:14 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-01-26 19:55 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: dwarves, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
>
> Add arguments to steal and thread_exit callbacks of conf_load to
> receive per-thread data.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> ---
Please carry over acks you got on previous revisions, unless you did
some drastic changes to already acked patches.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> btf_loader.c | 2 +-
> ctf_loader.c | 2 +-
> dwarf_loader.c | 4 ++--
> dwarves.h | 5 +++--
> pahole.c | 3 ++-
> pdwtags.c | 3 ++-
> pfunct.c | 4 +++-
> 7 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/btf_loader.c b/btf_loader.c
> index 7a5b16ff393e..b61cadd55127 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> * The app stole this cu, possibly deleting it,
> * so forget about it
> */
> - if (conf && conf->steal && conf->steal(cu, conf))
> + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> return 0;
>
> cus__add(cus, cu);
> diff --git a/ctf_loader.c b/ctf_loader.c
> index 7c34739afdce..de6d4dbfce97 100644
> --- a/ctf_loader.c
> +++ b/ctf_loader.c
> @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> * The app stole this cu, possibly deleting it,
> * so forget about it
> */
> - if (conf && conf->steal && conf->steal(cu, conf))
> + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> return 0;
>
> cus__add(cus, cu);
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index e30b03c1c541..bf9ea3765419 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> {
> cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> if (conf && conf->steal) {
> - return conf->steal(cu, conf);
> + return conf->steal(cu, conf, NULL);
> }
> return LSK__KEEPIT;
> }
> @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> goto out_abort;
> }
>
> - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> goto out_abort;
>
> return (void *)DWARF_CB_OK;
> diff --git a/dwarves.h b/dwarves.h
> index 52d162d67456..9a8e4de8843a 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -48,8 +48,9 @@ struct conf_fprintf;
> */
> struct conf_load {
> enum load_steal_kind (*steal)(struct cu *cu,
> - struct conf_load *conf);
> - int (*thread_exit)(void);
> + struct conf_load *conf,
> + void *thr_data);
> + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> void *cookie;
> char *format_path;
> int nr_jobs;
> diff --git a/pahole.c b/pahole.c
> index f3a51cb2fe74..f3eeaaca4cdf 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -2799,7 +2799,8 @@ out:
> static struct type_instance *header;
>
> static enum load_steal_kind pahole_stealer(struct cu *cu,
> - struct conf_load *conf_load)
> + struct conf_load *conf_load,
> + void *thr_data)
> {
> int ret = LSK__DELETE;
>
> diff --git a/pdwtags.c b/pdwtags.c
> index 2b5ba1bf6745..8b1d6f1c96cb 100644
> --- a/pdwtags.c
> +++ b/pdwtags.c
> @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> }
>
> static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> - struct conf_load *conf_load __maybe_unused)
> + struct conf_load *conf_load __maybe_unused,
> + void *thr_data __maybe_unused)
> {
> cu__emit_tags(cu);
> return LSK__DELETE;
> diff --git a/pfunct.c b/pfunct.c
> index 5485622e639b..314915b774f4 100644
> --- a/pfunct.c
> +++ b/pfunct.c
> @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> return EXIT_SUCCESS;
> }
>
> -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> + struct conf_load *conf_load __maybe_unused,
> + void *thr_data __maybe_unused)
> {
>
> if (function_name) {
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to worker threads.
2022-01-26 19:20 ` [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to " Kui-Feng Lee
@ 2022-01-26 19:55 ` Andrii Nakryiko
0 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-01-26 19:55 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: dwarves, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
>
> Add interfaces to allow users of dwarf_loader to prepare and pass
> per-thread data to steal-functions running on worker threads.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> dwarf_loader.c | 59 +++++++++++++++++++++++++++++++++++++++-----------
> dwarves.h | 4 ++++
> 2 files changed, 50 insertions(+), 13 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-26 19:20 ` [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking Kui-Feng Lee
@ 2022-01-26 19:58 ` Andrii Nakryiko
2022-01-26 20:57 ` Kui-Feng Lee
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-01-26 19:58 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: dwarves, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
>
> Create an instance of btf for each worker thread, and add type info to
> the local btf instance in the steal-function of pahole without mutex
> acquiring. Once finished with all worker threads, merge all
> per-thread btf instances to the primary btf instance.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> ---
There are still unnecessary casts and missing {} in the else branch,
but I'll let Arnaldo decide or fix it up.
Once this lands, can you please send kernel patch to use -j if pahole
support it during the kernel build? See scripts/pahole-version.sh and
scripts/link-vmlinux.sh for how pahole is set up and used in the
kernel. Thanks!
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> btf_encoder.c | 5 ++
> btf_encoder.h | 2 +
> pahole.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 124 insertions(+), 8 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision.
2022-01-26 19:20 ` [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision Kui-Feng Lee
@ 2022-01-26 19:59 ` Andrii Nakryiko
0 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-01-26 19:59 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: dwarves, Arnaldo Carvalho de Melo, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
>
> Replace deprecated APIs with new ones.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> btf_encoder.c | 20 ++++++++++----------
> btf_loader.c | 2 +-
> lib/bpf | 2 +-
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-01-26 19:55 ` Andrii Nakryiko
@ 2022-01-26 20:14 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 20:14 UTC (permalink / raw)
To: andrii.nakryiko; +Cc: daniel, ast, arnaldo.melo, andrii, dwarves, bpf
On Wed, 2022-01-26 at 11:55 -0800, Andrii Nakryiko wrote:
> On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> >
> > Add arguments to steal and thread_exit callbacks of conf_load to
> > receive per-thread data.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
>
> Please carry over acks you got on previous revisions, unless you did
> some drastic changes to already acked patches.
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
Sorry for that! It is my first time receiving acks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-26 19:58 ` Andrii Nakryiko
@ 2022-01-26 20:57 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
2022-01-27 20:12 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 25+ messages in thread
From: Kui-Feng Lee @ 2022-01-26 20:57 UTC (permalink / raw)
To: andrii.nakryiko; +Cc: daniel, ast, arnaldo.melo, andrii, dwarves, bpf
On Wed, 2022-01-26 at 11:58 -0800, Andrii Nakryiko wrote:
> On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> >
> > Create an instance of btf for each worker thread, and add type info
> > to
> > the local btf instance in the steal-function of pahole without
> > mutex
> > acquiring. Once finished with all worker threads, merge all
> > per-thread btf instances to the primary btf instance.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
>
> There are still unnecessary casts and missing {} in the else branch,
> but I'll let Arnaldo decide or fix it up.
>
> Once this lands, can you please send kernel patch to use -j if pahole
> support it during the kernel build? See scripts/pahole-version.sh and
> scripts/link-vmlinux.sh for how pahole is set up and used in the
> kernel. Thanks!
Sure!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-26 19:58 ` Andrii Nakryiko
2022-01-26 20:57 ` Kui-Feng Lee
@ 2022-01-27 10:05 ` Arnaldo Carvalho de Melo
2022-01-27 20:12 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-27 10:05 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Kui-Feng Lee, dwarves, Arnaldo Carvalho de Melo,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Wed, Jan 26, 2022 at 11:58:27AM -0800, Andrii Nakryiko escreveu:
> On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> >
> > Create an instance of btf for each worker thread, and add type info to
> > the local btf instance in the steal-function of pahole without mutex
> > acquiring. Once finished with all worker threads, merge all
> > per-thread btf instances to the primary btf instance.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
>
> There are still unnecessary casts and missing {} in the else branch,
> but I'll let Arnaldo decide or fix it up.
Yeah, I'll do some sanitization, thanks for all the review!
- Arnaldo
> Once this lands, can you please send kernel patch to use -j if pahole
> support it during the kernel build? See scripts/pahole-version.sh and
> scripts/link-vmlinux.sh for how pahole is set up and used in the
> kernel. Thanks!
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > btf_encoder.c | 5 ++
> > btf_encoder.h | 2 +
> > pahole.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++----
> > 3 files changed, 124 insertions(+), 8 deletions(-)
> >
>
> [...]
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 20:14 ` Kui-Feng Lee
@ 2022-01-27 10:05 ` Arnaldo Carvalho de Melo
2022-03-08 23:45 ` Andrii Nakryiko
1 sibling, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-27 10:05 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Kui-Feng Lee, dwarves, Arnaldo Carvalho de Melo,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> >
> > Add arguments to steal and thread_exit callbacks of conf_load to
> > receive per-thread data.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
>
> Please carry over acks you got on previous revisions, unless you did
> some drastic changes to already acked patches.
Yes, please do so.
I'll collect them this time, no need to resend.
- Arnaldo
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > btf_loader.c | 2 +-
> > ctf_loader.c | 2 +-
> > dwarf_loader.c | 4 ++--
> > dwarves.h | 5 +++--
> > pahole.c | 3 ++-
> > pdwtags.c | 3 ++-
> > pfunct.c | 4 +++-
> > 7 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/btf_loader.c b/btf_loader.c
> > index 7a5b16ff393e..b61cadd55127 100644
> > --- a/btf_loader.c
> > +++ b/btf_loader.c
> > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > * The app stole this cu, possibly deleting it,
> > * so forget about it
> > */
> > - if (conf && conf->steal && conf->steal(cu, conf))
> > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > return 0;
> >
> > cus__add(cus, cu);
> > diff --git a/ctf_loader.c b/ctf_loader.c
> > index 7c34739afdce..de6d4dbfce97 100644
> > --- a/ctf_loader.c
> > +++ b/ctf_loader.c
> > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > * The app stole this cu, possibly deleting it,
> > * so forget about it
> > */
> > - if (conf && conf->steal && conf->steal(cu, conf))
> > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > return 0;
> >
> > cus__add(cus, cu);
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index e30b03c1c541..bf9ea3765419 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > {
> > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > if (conf && conf->steal) {
> > - return conf->steal(cu, conf);
> > + return conf->steal(cu, conf, NULL);
> > }
> > return LSK__KEEPIT;
> > }
> > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > goto out_abort;
> > }
> >
> > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > goto out_abort;
> >
> > return (void *)DWARF_CB_OK;
> > diff --git a/dwarves.h b/dwarves.h
> > index 52d162d67456..9a8e4de8843a 100644
> > --- a/dwarves.h
> > +++ b/dwarves.h
> > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > */
> > struct conf_load {
> > enum load_steal_kind (*steal)(struct cu *cu,
> > - struct conf_load *conf);
> > - int (*thread_exit)(void);
> > + struct conf_load *conf,
> > + void *thr_data);
> > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > void *cookie;
> > char *format_path;
> > int nr_jobs;
> > diff --git a/pahole.c b/pahole.c
> > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -2799,7 +2799,8 @@ out:
> > static struct type_instance *header;
> >
> > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > - struct conf_load *conf_load)
> > + struct conf_load *conf_load,
> > + void *thr_data)
> > {
> > int ret = LSK__DELETE;
> >
> > diff --git a/pdwtags.c b/pdwtags.c
> > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > --- a/pdwtags.c
> > +++ b/pdwtags.c
> > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > }
> >
> > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > - struct conf_load *conf_load __maybe_unused)
> > + struct conf_load *conf_load __maybe_unused,
> > + void *thr_data __maybe_unused)
> > {
> > cu__emit_tags(cu);
> > return LSK__DELETE;
> > diff --git a/pfunct.c b/pfunct.c
> > index 5485622e639b..314915b774f4 100644
> > --- a/pfunct.c
> > +++ b/pfunct.c
> > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > return EXIT_SUCCESS;
> > }
> >
> > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > + struct conf_load *conf_load __maybe_unused,
> > + void *thr_data __maybe_unused)
> > {
> >
> > if (function_name) {
> > --
> > 2.30.2
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-26 19:58 ` Andrii Nakryiko
2022-01-26 20:57 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
@ 2022-01-27 20:12 ` Arnaldo Carvalho de Melo
2022-01-28 19:50 ` Arnaldo Carvalho de Melo
2 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-27 20:12 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Kui-Feng Lee, dwarves, Arnaldo Carvalho de Melo,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Wed, Jan 26, 2022 at 11:58:27AM -0800, Andrii Nakryiko escreveu:
> On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> >
> > Create an instance of btf for each worker thread, and add type info to
> > the local btf instance in the steal-function of pahole without mutex
> > acquiring. Once finished with all worker threads, merge all
> > per-thread btf instances to the primary btf instance.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
>
> There are still unnecessary casts and missing {} in the else branch,
> but I'll let Arnaldo decide or fix it up.
>
> Once this lands, can you please send kernel patch to use -j if pahole
> support it during the kernel build? See scripts/pahole-version.sh and
> scripts/link-vmlinux.sh for how pahole is set up and used in the
> kernel. Thanks!
I also tweaked this:
diff --git a/pahole.c b/pahole.c
index 8dcd6bf951fe1f93..1b2b19b2be45d30c 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2887,13 +2887,13 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
static pthread_mutex_t btf_lock = PTHREAD_MUTEX_INITIALIZER;
struct btf_encoder *encoder;
+ pthread_mutex_lock(&btf_lock);
/*
* FIXME:
*
* This should be really done at main(), but since in the current codebase only at this
* point we'll have cu->elf setup...
*/
- pthread_mutex_lock(&btf_lock);
if (!btf_encoder) {
/*
* btf_encoder is the primary encoder.
⬢[acme@toolbox pahole]$
As moving it to after that comment will only make the patch a bit
larger, changing nothing.
+++ b/pahole.c
@@ -2900,11 +2900,9 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
* And, it is used by the thread
* create it.
*/
- btf_encoder = btf_encoder__new(cu, detached_btf_filename,
- conf_load->base_btf,
- skip_encoding_btf_vars,
- btf_encode_force,
- btf_gen_floats, global_verbose);
+ btf_encoder = btf_encoder__new(cu, detached_btf_filename, conf_load->base_btf, skip_encoding_btf_vars,
+ btf_encode_force, btf_gen_floats, global_verbose);
+
if (btf_encoder && thr_data) {
struct thread_data *thread = thr_data;
⬢[acme@toolbox pahole]$
i.e. cosmetic stuff to make the patch smaller by keeping preexisting
lines as-is.
And the missing {} Andrii noticed:
diff --git a/pahole.c b/pahole.c
index 7e2e37582f21c566..8c0a982f05c9ae3d 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2937,8 +2937,9 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
thread->btf = btf_encoder__btf(thread->encoder);
}
encoder = thread->encoder;
- } else
+ } else {
encoder = btf_encoder;
+ }
if (btf_encoder__encode_cu(encoder, cu)) {
fprintf(stderr, "Encountered error while encoding BTF.\n");
⬢[acme@toolbox pahole]$
I'll look at the needless casts and push it to the 'next' and tmp.master
branches so that libbpf's CI can have a chance to test it.
I also added 'perf stat' results for 1.21 (the one in this fedora 34
workstation), 1.23 (parallel DWARF loading) and with your patches + the
libbpf update as a committer testing section, please add performance
numbers in in future work.
Thanks, applied!
- Arnaldo
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-27 20:12 ` Arnaldo Carvalho de Melo
@ 2022-01-28 19:50 ` Arnaldo Carvalho de Melo
2022-02-01 6:56 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-28 19:50 UTC (permalink / raw)
To: Andrii Nakryiko, Mark Wielaard
Cc: Kui-Feng Lee, dwarves, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, bpf
Em Thu, Jan 27, 2022 at 05:12:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jan 26, 2022 at 11:58:27AM -0800, Andrii Nakryiko escreveu:
> > On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > Create an instance of btf for each worker thread, and add type info to
> > > the local btf instance in the steal-function of pahole without mutex
> > > acquiring. Once finished with all worker threads, merge all
> > > per-thread btf instances to the primary btf instance.
> > There are still unnecessary casts and missing {} in the else branch,
> > but I'll let Arnaldo decide or fix it up.
So its just one unneeded cast as thr_data here is just a 'void *':
diff --git a/pahole.c b/pahole.c
index 8c0a982f05c9ae3d..39e18804100dbfda 100644
--- a/pahole.c
+++ b/pahole.c
@@ -2924,7 +2924,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
* avoids copying the data collected by the first thread.
*/
if (thr_data) {
- struct thread_data *thread = (struct thread_data *)thr_data;
+ struct thread_data *thread = thr_data;
if (thread->encoder == NULL) {
thread->encoder =
This other is needed as it is a "void **":
@@ -2832,7 +2832,7 @@ static int pahole_thread_exit(struct conf_load *conf, void *thr_data)
static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void **thr_data,
int error)
{
- struct thread_data **threads = (struct thread_data **)thr_data;
+ struct thread_data **threads = thr_data;
int i;
int err = 0;
Removing it:
/var/home/acme/git/pahole/pahole.c: In function ‘pahole_threads_collect’:
/var/home/acme/git/pahole/pahole.c:2835:40: warning: initialization of ‘struct thread_data **’ from incompatible pointer type ‘void **’ [-Wincompatible-pointer-types]
2835 | struct thread_data **threads = thr_data;
| ^~~~~~~~
And I did some more profiling, now the focus should go to elfutils:
⬢[acme@toolbox pahole]$ perf report --no-children -s dso --call-graph none 2> /dev/null | head -20
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 27K of event 'cycles:u'
# Event count (approx.): 27956766207
#
# Overhead Shared Object
# ........ ...................
#
46.70% libdwarves.so.1.0.0
39.84% libdw-0.186.so
9.70% libc-2.33.so
2.14% libpthread-2.33.so
1.47% [unknown]
0.09% ld-2.33.so
0.06% libelf-0.186.so
0.00% libcrypto.so.1.1.1l
0.00% libk5crypto.so.3.1
⬢[acme@toolbox pahole]$
$ perf report -g graph,0.5,2 --stdio --no-children -s dso --dso libdw-0.186.so
# To display the perf.data header info, please use --header/--header-only options.
#
# dso: libdw-0.186.so
#
# Total Lost Samples: 0
#
# Samples: 27K of event 'cycles:u'
# Event count (approx.): 27956766207
#
# Overhead Shared Object
# ........ ..............
#
39.84% libdw-0.186.so
|
|--25.66%--__libdw_find_attr
| |
| |--20.96%--__dwarf_attr_internal (inlined)
| | |
| | |--10.94%--attr_numeric
| | | |
| | | |--9.96%--die__process_class
| | | | __die__process_tag
| | | | die__process_unit
| | | | die__process
| | | | dwarf_cus__create_and_process_cu
| | | | dwarf_cus__process_cu_thread
| | | | start_thread
| | | | __GI___clone (inlined)
| | | |
| | | --0.61%--type__init
| | | __die__process_tag
| | | die__process_unit
| | | die__process
| | | dwarf_cus__create_and_process_cu
| | | dwarf_cus__process_cu_thread
| | | start_thread
| | | __GI___clone (inlined)
| | |
| | |--5.43%--attr_type
| | | |
| | | --4.94%--tag__init
| | | |
| | | |--2.60%--die__process_class
| | | | __die__process_tag
| | | | die__process_unit
| | | | die__process
| | | | dwarf_cus__create_and_process_cu
| | | | dwarf_cus__process_cu_thread
| | | | start_thread
| | | | __GI___clone (inlined)
| | | |
| | | |--0.99%--__die__process_tag
| | | | |
| | | | --0.98%--die__process_unit
| | | | die__process
| | | | dwarf_cus__create_and_process_cu
| | | | dwarf_cus__process_cu_thread
| | | | start_thread
| | | | __GI___clone (inlined)
| |
| |--4.01%--__dwarf_siblingof_internal (inlined)
| | |
| | |--1.41%--die__process_class
| | | __die__process_tag
| | | die__process_unit
| | | die__process
| | | dwarf_cus__create_and_process_cu
| | | dwarf_cus__process_cu_thread
| | | start_thread
| | | __GI___clone (inlined)
| | |
| | |--1.02%--die__process
| | | dwarf_cus__create_and_process_cu
| | | dwarf_cus__process_cu_thread
| | | start_thread
| | | __GI___clone (inlined)
|
|--2.38%--__libdw_form_val_compute_len
| __libdw_find_attr
| |
| --1.86%--__dwarf_attr_internal (inlined)
| |
| |--0.94%--attr_numeric
| | |
| | --0.83%--die__process_class
| | __die__process_tag
| | die__process_unit
| | die__process
| | dwarf_cus__create_and_process_cu
| | dwarf_cus__process_cu_thread
| | start_thread
| | __GI___clone (inlined)
| |
| --0.56%--attr_type
| |
| --0.53%--tag__init
#
# (Tip: If you have debuginfo enabled, try: perf report -s sym,srcline)
#
This find_attr thing needs improvements, its a linear search AFAIK, some
hashtable could do wonders, I guess.
Mark, was this considered at some point?
⬢[acme@toolbox pahole]$ rpm -q elfutils-libs
elfutils-libs-0.186-1.fc34.x86_64
Andrii https://github.com/libbpf/libbpf/actions/workflows/pahole.yml is
in failure mode for 3 days, and only yesterday I pushed these changes,
seems unrelated to pahole:
Tests exit status: 1
Test Results:
bpftool: PASS
test_progs: FAIL (returned 1)
test_progs-no_alu32: FAIL (returned 1)
test_verifier: PASS
shutdown: CLEAN
Error: Process completed with exit code 1.
Can you please check?
- Arnaldo
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-01-28 19:50 ` Arnaldo Carvalho de Melo
@ 2022-02-01 6:56 ` Andrii Nakryiko
2022-02-02 0:16 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2022-02-01 6:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Mark Wielaard, Kui-Feng Lee, dwarves, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, bpf
On Fri, Jan 28, 2022 at 11:52 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Jan 27, 2022 at 05:12:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Jan 26, 2022 at 11:58:27AM -0800, Andrii Nakryiko escreveu:
> > > On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > > Create an instance of btf for each worker thread, and add type info to
> > > > the local btf instance in the steal-function of pahole without mutex
> > > > acquiring. Once finished with all worker threads, merge all
> > > > per-thread btf instances to the primary btf instance.
>
> > > There are still unnecessary casts and missing {} in the else branch,
> > > but I'll let Arnaldo decide or fix it up.
>
> So its just one unneeded cast as thr_data here is just a 'void *':
>
> diff --git a/pahole.c b/pahole.c
> index 8c0a982f05c9ae3d..39e18804100dbfda 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -2924,7 +2924,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
> * avoids copying the data collected by the first thread.
> */
> if (thr_data) {
> - struct thread_data *thread = (struct thread_data *)thr_data;
> + struct thread_data *thread = thr_data;
>
> if (thread->encoder == NULL) {
> thread->encoder =
>
>
> This other is needed as it is a "void **":
>
> @@ -2832,7 +2832,7 @@ static int pahole_thread_exit(struct conf_load *conf, void *thr_data)
> static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void **thr_data,
> int error)
> {
> - struct thread_data **threads = (struct thread_data **)thr_data;
> + struct thread_data **threads = thr_data;
> int i;
> int err = 0;
>
>
> Removing it:
>
> /var/home/acme/git/pahole/pahole.c: In function ‘pahole_threads_collect’:
> /var/home/acme/git/pahole/pahole.c:2835:40: warning: initialization of ‘struct thread_data **’ from incompatible pointer type ‘void **’ [-Wincompatible-pointer-types]
> 2835 | struct thread_data **threads = thr_data;
> | ^~~~~~~~
>
>
> And I did some more profiling, now the focus should go to elfutils:
>
> ⬢[acme@toolbox pahole]$ perf report --no-children -s dso --call-graph none 2> /dev/null | head -20
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 27K of event 'cycles:u'
> # Event count (approx.): 27956766207
> #
> # Overhead Shared Object
> # ........ ...................
> #
> 46.70% libdwarves.so.1.0.0
> 39.84% libdw-0.186.so
> 9.70% libc-2.33.so
> 2.14% libpthread-2.33.so
> 1.47% [unknown]
> 0.09% ld-2.33.so
> 0.06% libelf-0.186.so
> 0.00% libcrypto.so.1.1.1l
> 0.00% libk5crypto.so.3.1
> ⬢[acme@toolbox pahole]$
>
> $ perf report -g graph,0.5,2 --stdio --no-children -s dso --dso libdw-0.186.so
>
[...]
>
> #
> # (Tip: If you have debuginfo enabled, try: perf report -s sym,srcline)
> #
>
> This find_attr thing needs improvements, its a linear search AFAIK, some
> hashtable could do wonders, I guess.
>
> Mark, was this considered at some point?
This would be a great improvement, yes!
But strange that you didn't see any BTF-related functions, are you
sure you profiled the entire DWARF to BTF conversion process? BTF
encoding is not dominant, but noticeable anyways (e.g., adding unique
strings to BTF is relatively expensive still).
>
> ⬢[acme@toolbox pahole]$ rpm -q elfutils-libs
> elfutils-libs-0.186-1.fc34.x86_64
>
> Andrii https://github.com/libbpf/libbpf/actions/workflows/pahole.yml is
> in failure mode for 3 days, and only yesterday I pushed these changes,
> seems unrelated to pahole:
>
> Tests exit status: 1
> Test Results:
> bpftool: PASS
> test_progs: FAIL (returned 1)
> test_progs-no_alu32: FAIL (returned 1)
> test_verifier: PASS
> shutdown: CLEAN
> Error: Process completed with exit code 1.
>
> Can you please check?
Yes, it's not related to pahole. This is the BPF selftests issue which
I already fixed last week, but didn't get a chance to sync everything
to Github repo before leaving for a short vacation. I'll do another
sync tonight and it should be all green again.
>
> - Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking.
2022-02-01 6:56 ` Andrii Nakryiko
@ 2022-02-02 0:16 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-02 0:16 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Arnaldo Carvalho de Melo, Mark Wielaard, Kui-Feng Lee, dwarves,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Mon, Jan 31, 2022 at 10:56:16PM -0800, Andrii Nakryiko escreveu:
> On Fri, Jan 28, 2022 at 11:52 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Thu, Jan 27, 2022 at 05:12:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Wed, Jan 26, 2022 at 11:58:27AM -0800, Andrii Nakryiko escreveu:
> > > > On Wed, Jan 26, 2022 at 11:21 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > > > Create an instance of btf for each worker thread, and add type info to
> > > > > the local btf instance in the steal-function of pahole without mutex
> > > > > acquiring. Once finished with all worker threads, merge all
> > > > > per-thread btf instances to the primary btf instance.
> >
> > > > There are still unnecessary casts and missing {} in the else branch,
> > > > but I'll let Arnaldo decide or fix it up.
> >
> > So its just one unneeded cast as thr_data here is just a 'void *':
> >
> > diff --git a/pahole.c b/pahole.c
> > index 8c0a982f05c9ae3d..39e18804100dbfda 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -2924,7 +2924,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu,
> > * avoids copying the data collected by the first thread.
> > */
> > if (thr_data) {
> > - struct thread_data *thread = (struct thread_data *)thr_data;
> > + struct thread_data *thread = thr_data;
> >
> > if (thread->encoder == NULL) {
> > thread->encoder =
> >
> >
> > This other is needed as it is a "void **":
> >
> > @@ -2832,7 +2832,7 @@ static int pahole_thread_exit(struct conf_load *conf, void *thr_data)
> > static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void **thr_data,
> > int error)
> > {
> > - struct thread_data **threads = (struct thread_data **)thr_data;
> > + struct thread_data **threads = thr_data;
> > int i;
> > int err = 0;
> >
> >
> > Removing it:
> >
> > /var/home/acme/git/pahole/pahole.c: In function ‘pahole_threads_collect’:
> > /var/home/acme/git/pahole/pahole.c:2835:40: warning: initialization of ‘struct thread_data **’ from incompatible pointer type ‘void **’ [-Wincompatible-pointer-types]
> > 2835 | struct thread_data **threads = thr_data;
> > | ^~~~~~~~
> >
> >
> > And I did some more profiling, now the focus should go to elfutils:
> >
> > ⬢[acme@toolbox pahole]$ perf report --no-children -s dso --call-graph none 2> /dev/null | head -20
> > # To display the perf.data header info, please use --header/--header-only options.
> > #
> > #
> > # Total Lost Samples: 0
> > #
> > # Samples: 27K of event 'cycles:u'
> > # Event count (approx.): 27956766207
> > #
> > # Overhead Shared Object
> > # ........ ...................
> > #
> > 46.70% libdwarves.so.1.0.0
> > 39.84% libdw-0.186.so
> > 9.70% libc-2.33.so
> > 2.14% libpthread-2.33.so
> > 1.47% [unknown]
> > 0.09% ld-2.33.so
> > 0.06% libelf-0.186.so
> > 0.00% libcrypto.so.1.1.1l
> > 0.00% libk5crypto.so.3.1
> > ⬢[acme@toolbox pahole]$
> >
> > $ perf report -g graph,0.5,2 --stdio --no-children -s dso --dso libdw-0.186.so
> >
>
> [...]
>
> >
> > #
> > # (Tip: If you have debuginfo enabled, try: perf report -s sym,srcline)
> > #
> >
> > This find_attr thing needs improvements, its a linear search AFAIK, some
> > hashtable could do wonders, I guess.
> >
> > Mark, was this considered at some point?
>
> This would be a great improvement, yes!
>
> But strange that you didn't see any BTF-related functions, are you
> sure you profiled the entire DWARF to BTF conversion process? BTF
> encoding is not dominant, but noticeable anyways (e.g., adding unique
> strings to BTF is relatively expensive still).
It appears under the
> > 46.70% libdwarves.so.1.0.0
line, since we statically link libbpf. So yeah, it was unfortunate the
way I presented the profiling output, I was just trying to point to what
I think is the low hanging fruit, i.e. optimizing find_attr routines in
libdw-0.186.so.
> >
> > ⬢[acme@toolbox pahole]$ rpm -q elfutils-libs
> > elfutils-libs-0.186-1.fc34.x86_64
> >
> > Andrii https://github.com/libbpf/libbpf/actions/workflows/pahole.yml is
> > in failure mode for 3 days, and only yesterday I pushed these changes,
> > seems unrelated to pahole:
> >
> > Tests exit status: 1
> > Test Results:
> > bpftool: PASS
> > test_progs: FAIL (returned 1)
> > test_progs-no_alu32: FAIL (returned 1)
> > test_verifier: PASS
> > shutdown: CLEAN
> > Error: Process completed with exit code 1.
> >
> > Can you please check?
>
> Yes, it's not related to pahole. This is the BPF selftests issue which
> I already fixed last week, but didn't get a chance to sync everything
> to Github repo before leaving for a short vacation. I'll do another
> sync tonight and it should be all green again.
I'll check tomorrow.
Going green I'll do some more tests and work towards releasing 1.24.
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
@ 2022-03-08 23:45 ` Andrii Nakryiko
2022-03-09 19:24 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2022-03-08 23:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Kui-Feng Lee, dwarves, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, bpf
On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > >
> > > Add arguments to steal and thread_exit callbacks of conf_load to
> > > receive per-thread data.
> > >
> > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > ---
> >
> > Please carry over acks you got on previous revisions, unless you did
> > some drastic changes to already acked patches.
>
> Yes, please do so.
>
> I'll collect them this time, no need to resend.
>
Hey, Arnaldo!
Any idea when these patches will make it into master branch? I see
they are in tmp.master right now.
> - Arnaldo
>
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > > btf_loader.c | 2 +-
> > > ctf_loader.c | 2 +-
> > > dwarf_loader.c | 4 ++--
> > > dwarves.h | 5 +++--
> > > pahole.c | 3 ++-
> > > pdwtags.c | 3 ++-
> > > pfunct.c | 4 +++-
> > > 7 files changed, 14 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/btf_loader.c b/btf_loader.c
> > > index 7a5b16ff393e..b61cadd55127 100644
> > > --- a/btf_loader.c
> > > +++ b/btf_loader.c
> > > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > > * The app stole this cu, possibly deleting it,
> > > * so forget about it
> > > */
> > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > return 0;
> > >
> > > cus__add(cus, cu);
> > > diff --git a/ctf_loader.c b/ctf_loader.c
> > > index 7c34739afdce..de6d4dbfce97 100644
> > > --- a/ctf_loader.c
> > > +++ b/ctf_loader.c
> > > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > > * The app stole this cu, possibly deleting it,
> > > * so forget about it
> > > */
> > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > return 0;
> > >
> > > cus__add(cus, cu);
> > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > index e30b03c1c541..bf9ea3765419 100644
> > > --- a/dwarf_loader.c
> > > +++ b/dwarf_loader.c
> > > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > > {
> > > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > > if (conf && conf->steal) {
> > > - return conf->steal(cu, conf);
> > > + return conf->steal(cu, conf, NULL);
> > > }
> > > return LSK__KEEPIT;
> > > }
> > > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > > goto out_abort;
> > > }
> > >
> > > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > > goto out_abort;
> > >
> > > return (void *)DWARF_CB_OK;
> > > diff --git a/dwarves.h b/dwarves.h
> > > index 52d162d67456..9a8e4de8843a 100644
> > > --- a/dwarves.h
> > > +++ b/dwarves.h
> > > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > > */
> > > struct conf_load {
> > > enum load_steal_kind (*steal)(struct cu *cu,
> > > - struct conf_load *conf);
> > > - int (*thread_exit)(void);
> > > + struct conf_load *conf,
> > > + void *thr_data);
> > > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > > void *cookie;
> > > char *format_path;
> > > int nr_jobs;
> > > diff --git a/pahole.c b/pahole.c
> > > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > > --- a/pahole.c
> > > +++ b/pahole.c
> > > @@ -2799,7 +2799,8 @@ out:
> > > static struct type_instance *header;
> > >
> > > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > > - struct conf_load *conf_load)
> > > + struct conf_load *conf_load,
> > > + void *thr_data)
> > > {
> > > int ret = LSK__DELETE;
> > >
> > > diff --git a/pdwtags.c b/pdwtags.c
> > > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > > --- a/pdwtags.c
> > > +++ b/pdwtags.c
> > > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > > }
> > >
> > > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > > - struct conf_load *conf_load __maybe_unused)
> > > + struct conf_load *conf_load __maybe_unused,
> > > + void *thr_data __maybe_unused)
> > > {
> > > cu__emit_tags(cu);
> > > return LSK__DELETE;
> > > diff --git a/pfunct.c b/pfunct.c
> > > index 5485622e639b..314915b774f4 100644
> > > --- a/pfunct.c
> > > +++ b/pfunct.c
> > > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > > return EXIT_SUCCESS;
> > > }
> > >
> > > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > > + struct conf_load *conf_load __maybe_unused,
> > > + void *thr_data __maybe_unused)
> > > {
> > >
> > > if (function_name) {
> > > --
> > > 2.30.2
> > >
>
> --
>
> - Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-08 23:45 ` Andrii Nakryiko
@ 2022-03-09 19:24 ` Arnaldo Carvalho de Melo
2022-03-10 0:14 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-09 19:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Arnaldo Carvalho de Melo, Kui-Feng Lee, dwarves,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Tue, Mar 08, 2022 at 03:45:03PM -0800, Andrii Nakryiko escreveu:
> On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> > > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > >
> > > > Add arguments to steal and thread_exit callbacks of conf_load to
> > > > receive per-thread data.
> > > >
> > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > ---
> > >
> > > Please carry over acks you got on previous revisions, unless you did
> > > some drastic changes to already acked patches.
> >
> > Yes, please do so.
> >
> > I'll collect them this time, no need to resend.
> >
>
> Hey, Arnaldo!
>
> Any idea when these patches will make it into master branch? I see
> they are in tmp.master right now.
I did some minor fixups to the cset comment and to the code in the
'pahole --compile' new feature at the head of it and pushed all up,
please check.
- Arnaldo
> > - Arnaldo
> >
> > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > >
> > > > btf_loader.c | 2 +-
> > > > ctf_loader.c | 2 +-
> > > > dwarf_loader.c | 4 ++--
> > > > dwarves.h | 5 +++--
> > > > pahole.c | 3 ++-
> > > > pdwtags.c | 3 ++-
> > > > pfunct.c | 4 +++-
> > > > 7 files changed, 14 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/btf_loader.c b/btf_loader.c
> > > > index 7a5b16ff393e..b61cadd55127 100644
> > > > --- a/btf_loader.c
> > > > +++ b/btf_loader.c
> > > > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > > > * The app stole this cu, possibly deleting it,
> > > > * so forget about it
> > > > */
> > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > return 0;
> > > >
> > > > cus__add(cus, cu);
> > > > diff --git a/ctf_loader.c b/ctf_loader.c
> > > > index 7c34739afdce..de6d4dbfce97 100644
> > > > --- a/ctf_loader.c
> > > > +++ b/ctf_loader.c
> > > > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > > > * The app stole this cu, possibly deleting it,
> > > > * so forget about it
> > > > */
> > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > return 0;
> > > >
> > > > cus__add(cus, cu);
> > > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > > index e30b03c1c541..bf9ea3765419 100644
> > > > --- a/dwarf_loader.c
> > > > +++ b/dwarf_loader.c
> > > > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > > > {
> > > > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > > > if (conf && conf->steal) {
> > > > - return conf->steal(cu, conf);
> > > > + return conf->steal(cu, conf, NULL);
> > > > }
> > > > return LSK__KEEPIT;
> > > > }
> > > > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > > > goto out_abort;
> > > > }
> > > >
> > > > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > > > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > > > goto out_abort;
> > > >
> > > > return (void *)DWARF_CB_OK;
> > > > diff --git a/dwarves.h b/dwarves.h
> > > > index 52d162d67456..9a8e4de8843a 100644
> > > > --- a/dwarves.h
> > > > +++ b/dwarves.h
> > > > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > > > */
> > > > struct conf_load {
> > > > enum load_steal_kind (*steal)(struct cu *cu,
> > > > - struct conf_load *conf);
> > > > - int (*thread_exit)(void);
> > > > + struct conf_load *conf,
> > > > + void *thr_data);
> > > > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > > > void *cookie;
> > > > char *format_path;
> > > > int nr_jobs;
> > > > diff --git a/pahole.c b/pahole.c
> > > > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > > > --- a/pahole.c
> > > > +++ b/pahole.c
> > > > @@ -2799,7 +2799,8 @@ out:
> > > > static struct type_instance *header;
> > > >
> > > > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > > > - struct conf_load *conf_load)
> > > > + struct conf_load *conf_load,
> > > > + void *thr_data)
> > > > {
> > > > int ret = LSK__DELETE;
> > > >
> > > > diff --git a/pdwtags.c b/pdwtags.c
> > > > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > > > --- a/pdwtags.c
> > > > +++ b/pdwtags.c
> > > > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > > > }
> > > >
> > > > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > > > - struct conf_load *conf_load __maybe_unused)
> > > > + struct conf_load *conf_load __maybe_unused,
> > > > + void *thr_data __maybe_unused)
> > > > {
> > > > cu__emit_tags(cu);
> > > > return LSK__DELETE;
> > > > diff --git a/pfunct.c b/pfunct.c
> > > > index 5485622e639b..314915b774f4 100644
> > > > --- a/pfunct.c
> > > > +++ b/pfunct.c
> > > > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > > > return EXIT_SUCCESS;
> > > > }
> > > >
> > > > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > > > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > > > + struct conf_load *conf_load __maybe_unused,
> > > > + void *thr_data __maybe_unused)
> > > > {
> > > >
> > > > if (function_name) {
> > > > --
> > > > 2.30.2
> > > >
> >
> > --
> >
> > - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-09 19:24 ` Arnaldo Carvalho de Melo
@ 2022-03-10 0:14 ` Andrii Nakryiko
2022-03-10 0:18 ` Andrii Nakryiko
2022-03-14 15:38 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2022-03-10 0:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Arnaldo Carvalho de Melo, Kui-Feng Lee, dwarves,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Mar 9, 2022 at 11:24 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Mar 08, 2022 at 03:45:03PM -0800, Andrii Nakryiko escreveu:
> > On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> > > > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > > >
> > > > > Add arguments to steal and thread_exit callbacks of conf_load to
> > > > > receive per-thread data.
> > > > >
> > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > > ---
> > > >
> > > > Please carry over acks you got on previous revisions, unless you did
> > > > some drastic changes to already acked patches.
> > >
> > > Yes, please do so.
> > >
> > > I'll collect them this time, no need to resend.
> > >
> >
> > Hey, Arnaldo!
> >
> > Any idea when these patches will make it into master branch? I see
> > they are in tmp.master right now.
>
> I did some minor fixups to the cset comment and to the code in the
> 'pahole --compile' new feature at the head of it and pushed all up,
> please check.
>
I did check locally with latest pahole master, and it seems like
something is wrong with generated BTF. I get three selftests failure
if I use latest pahole compiled from master.
Kui-Feng, please take a look when you get a chance. Arnaldo, please
hold off from releasing a new version for now.
> - Arnaldo
>
> > > - Arnaldo
> > >
> > > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > > >
> > > > > btf_loader.c | 2 +-
> > > > > ctf_loader.c | 2 +-
> > > > > dwarf_loader.c | 4 ++--
> > > > > dwarves.h | 5 +++--
> > > > > pahole.c | 3 ++-
> > > > > pdwtags.c | 3 ++-
> > > > > pfunct.c | 4 +++-
> > > > > 7 files changed, 14 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/btf_loader.c b/btf_loader.c
> > > > > index 7a5b16ff393e..b61cadd55127 100644
> > > > > --- a/btf_loader.c
> > > > > +++ b/btf_loader.c
> > > > > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > > > > * The app stole this cu, possibly deleting it,
> > > > > * so forget about it
> > > > > */
> > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > return 0;
> > > > >
> > > > > cus__add(cus, cu);
> > > > > diff --git a/ctf_loader.c b/ctf_loader.c
> > > > > index 7c34739afdce..de6d4dbfce97 100644
> > > > > --- a/ctf_loader.c
> > > > > +++ b/ctf_loader.c
> > > > > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > > > > * The app stole this cu, possibly deleting it,
> > > > > * so forget about it
> > > > > */
> > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > return 0;
> > > > >
> > > > > cus__add(cus, cu);
> > > > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > > > index e30b03c1c541..bf9ea3765419 100644
> > > > > --- a/dwarf_loader.c
> > > > > +++ b/dwarf_loader.c
> > > > > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > > > > {
> > > > > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > > > > if (conf && conf->steal) {
> > > > > - return conf->steal(cu, conf);
> > > > > + return conf->steal(cu, conf, NULL);
> > > > > }
> > > > > return LSK__KEEPIT;
> > > > > }
> > > > > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > > > > goto out_abort;
> > > > > }
> > > > >
> > > > > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > > > > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > > > > goto out_abort;
> > > > >
> > > > > return (void *)DWARF_CB_OK;
> > > > > diff --git a/dwarves.h b/dwarves.h
> > > > > index 52d162d67456..9a8e4de8843a 100644
> > > > > --- a/dwarves.h
> > > > > +++ b/dwarves.h
> > > > > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > > > > */
> > > > > struct conf_load {
> > > > > enum load_steal_kind (*steal)(struct cu *cu,
> > > > > - struct conf_load *conf);
> > > > > - int (*thread_exit)(void);
> > > > > + struct conf_load *conf,
> > > > > + void *thr_data);
> > > > > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > > > > void *cookie;
> > > > > char *format_path;
> > > > > int nr_jobs;
> > > > > diff --git a/pahole.c b/pahole.c
> > > > > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > > > > --- a/pahole.c
> > > > > +++ b/pahole.c
> > > > > @@ -2799,7 +2799,8 @@ out:
> > > > > static struct type_instance *header;
> > > > >
> > > > > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > > > > - struct conf_load *conf_load)
> > > > > + struct conf_load *conf_load,
> > > > > + void *thr_data)
> > > > > {
> > > > > int ret = LSK__DELETE;
> > > > >
> > > > > diff --git a/pdwtags.c b/pdwtags.c
> > > > > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > > > > --- a/pdwtags.c
> > > > > +++ b/pdwtags.c
> > > > > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > > > > }
> > > > >
> > > > > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > > > > - struct conf_load *conf_load __maybe_unused)
> > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > + void *thr_data __maybe_unused)
> > > > > {
> > > > > cu__emit_tags(cu);
> > > > > return LSK__DELETE;
> > > > > diff --git a/pfunct.c b/pfunct.c
> > > > > index 5485622e639b..314915b774f4 100644
> > > > > --- a/pfunct.c
> > > > > +++ b/pfunct.c
> > > > > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > > > > return EXIT_SUCCESS;
> > > > > }
> > > > >
> > > > > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > > > > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > + void *thr_data __maybe_unused)
> > > > > {
> > > > >
> > > > > if (function_name) {
> > > > > --
> > > > > 2.30.2
> > > > >
> > >
> > > --
> > >
> > > - Arnaldo
>
> --
>
> - Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-10 0:14 ` Andrii Nakryiko
@ 2022-03-10 0:18 ` Andrii Nakryiko
2022-03-21 19:06 ` Kui-Feng Lee
2022-03-14 15:38 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2022-03-10 0:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Arnaldo Carvalho de Melo, Kui-Feng Lee, dwarves,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
On Wed, Mar 9, 2022 at 4:14 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Mar 9, 2022 at 11:24 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Tue, Mar 08, 2022 at 03:45:03PM -0800, Andrii Nakryiko escreveu:
> > > On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> > > > > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > > > >
> > > > > > Add arguments to steal and thread_exit callbacks of conf_load to
> > > > > > receive per-thread data.
> > > > > >
> > > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > > > ---
> > > > >
> > > > > Please carry over acks you got on previous revisions, unless you did
> > > > > some drastic changes to already acked patches.
> > > >
> > > > Yes, please do so.
> > > >
> > > > I'll collect them this time, no need to resend.
> > > >
> > >
> > > Hey, Arnaldo!
> > >
> > > Any idea when these patches will make it into master branch? I see
> > > they are in tmp.master right now.
> >
> > I did some minor fixups to the cset comment and to the code in the
> > 'pahole --compile' new feature at the head of it and pushed all up,
> > please check.
> >
>
> I did check locally with latest pahole master, and it seems like
> something is wrong with generated BTF. I get three selftests failure
> if I use latest pahole compiled from master.
>
> Kui-Feng, please take a look when you get a chance. Arnaldo, please
> hold off from releasing a new version for now.
Specifically:
libbpf: prog 'kfunc_call_test1': BPF program load failed: Permission denied
libbpf: prog 'kfunc_call_test1': -- BEGIN PROG LOAD LOG --
Validating f1() func#1...
2: R1=ctx(off=0,imm=0) R10=fp0
; int __noinline f1(struct __sk_buff *skb)
2: (b4) w7 = -1 ; R7_w=P4294967295
; struct bpf_sock *sk = skb->sk;
3: (79) r1 = *(u64 *)(r1 +168) ;
R1_w=sock_common_or_null(id=1,off=0,imm=0)
; if (!sk)
4: (15) if r1 == 0x0 goto pc+26 ; R1_w=sock_common(off=0,imm=0)
; sk = bpf_sk_fullsock(sk);
5: (85) call bpf_sk_fullsock#95 ; R0_w=sock_or_null(id=2,off=0,imm=0)
6: (bf) r6 = r0 ;
R0_w=sock_or_null(id=2,off=0,imm=0)
R6_w=sock_or_null(id=2,off=0,imm=0)
; if (!sk)
7: (15) if r6 == 0x0 goto pc+23 ; R6_w=sock(off=0,imm=0)
; bpf_get_smp_processor_id());
8: (85) call bpf_get_smp_processor_id#8 ; R0=Pscalar()
; active = (int *)bpf_per_cpu_ptr(&bpf_prog_active,
9: (18) r1 = 0x275e0 ; R1_w=rdonly_mem(off=0,imm=0)
11: (bc) w2 = w0 ; R0=Pscalar()
R2_w=Pscalar(umax=4294967295,var_off=(0x0; 0xffffffff))
12: (85) call bpf_per_cpu_ptr#153
R1 type=rdonly_mem expected=percpu_ptr_
processed 10 insns (limit 1000000) max_states_per_insn 0 total_states
1 peak_states 1 mark_read 1
-- END PROG LOAD LOG --
libbpf: failed to load program 'kfunc_call_test1'
libbpf: failed to load object 'kfunc_call_test_subprog'
libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -13
test_subprog:FAIL:skel unexpected error: -13
#77/2 kfunc_call/subprog:FAIL
test_subprog_lskel:FAIL:skel unexpected error: -13
#77/3 kfunc_call/subprog_lskel:FAIL
test_ksyms_btf:PASS:btf_exists 0 nsec
test_basic:PASS:kallsyms_fopen 0 nsec
test_basic:PASS:ksym_find 0 nsec
test_basic:PASS:kallsyms_fopen 0 nsec
test_basic:PASS:ksym_find 0 nsec
libbpf: prog 'handler': BPF program load failed: Permission denied
libbpf: prog 'handler': -- BEGIN PROG LOAD LOG --
arg#0 reference type('UNKNOWN ') size cannot be determined: -22
0: R1=ctx(off=0,imm=0) R10=fp0
; out__bpf_prog_active_addr = (__u64)&bpf_prog_active;
0: (18) r1 = 0xffffc9000071a008 ; R1_w=map_value(off=8,ks=4,vs=36,imm=0)
2: (18) r2 = 0x275e0 ; R2_w=rdonly_mem(off=0,imm=0)
4: (7b) *(u64 *)(r1 +0) = r2 ;
R1_w=map_value(off=8,ks=4,vs=36,imm=0) R2_w=rdonly_mem(off=0,imm=0)
; out__runqueues_addr = (__u64)&runqueues;
5: (18) r1 = 0xffffc9000071a000 ; R1_w=map_value(off=0,ks=4,vs=36,imm=0)
7: (18) r2 = 0x2b140 ; R2_w=ptr_rq(off=0,imm=0)
9: (7b) *(u64 *)(r1 +0) = r2 ;
R1_w=map_value(off=0,ks=4,vs=36,imm=0) R2_w=ptr_rq(off=0,imm=0)
; cpu = bpf_get_smp_processor_id();
10: (85) call bpf_get_smp_processor_id#8 ; R0_w=scalar()
11: (bc) w6 = w0 ; R0_w=scalar()
R6_w=scalar(umax=4294967295,var_off=(0x0; 0xffffffff))
; rq = (struct rq *)bpf_per_cpu_ptr(&runqueues, cpu);
12: (18) r1 = 0x2b140 ; R1_w=ptr_rq(off=0,imm=0)
14: (bc) w2 = w6 ;
R2_w=scalar(umax=4294967295,var_off=(0x0; 0xffffffff))
R6_w=scalar(umax=4294967295,var_off=(0x0; 0xffffffff))
15: (85) call bpf_per_cpu_ptr#153
R1 type=ptr_ expected=percpu_ptr_
processed 11 insns (limit 1000000) max_states_per_insn 0 total_states
0 peak_states 0 mark_read 0
-- END PROG LOAD LOG --
libbpf: failed to load program 'handler'
libbpf: failed to load object 'test_ksyms_btf'
libbpf: failed to load BPF skeleton 'test_ksyms_btf': -13
test_basic:FAIL:skel_open failed to open and load skeleton
#79/1 ksyms_btf/basic:FAIL
#80 ksyms_module:FAIL
test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
unexpected error: -13
#80/1 ksyms_module/lskel:FAIL
libbpf: prog 'load': BPF program load failed: Permission denied
libbpf: prog 'load': -- BEGIN PROG LOAD LOG --
0: R1=ctx(off=0,imm=0) R10=fp0
; if (x)
0: (18) r1 = 0xffffc9000109e000 ; R1_w=map_value(off=0,ks=4,vs=4,imm=0)
2: (61) r1 = *(u32 *)(r1 +0) ; R1_w=0
; if (x)
3: (16) if w1 == 0x0 goto pc+1 ; R1_w=P0
; bpf_testmod_test_mod_kfunc(42);
5: (b4) w1 = 42 ; R1_w=42
6: (85) call bpf_testmod_test_mod_kfunc#123710
; out_bpf_testmod_ksym = *(int *)bpf_this_cpu_ptr(&bpf_testmod_ksym_percpu);
7: (18) r1 = 0x2c3e8 ; R1_w=rdonly_mem(off=0,imm=0)
9: (85) call bpf_this_cpu_ptr#154
R1 type=rdonly_mem expected=percpu_ptr_
processed 7 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
-- END PROG LOAD LOG --
libbpf: failed to load program 'load'
libbpf: failed to load object 'test_ksyms_module'
libbpf: failed to load BPF skeleton 'test_ksyms_module': -13
test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -13
#80/2 ksyms_module/libbpf:FAIL
So it seems like something broke around per-CPU variable info generation.
>
>
> > - Arnaldo
> >
> > > > - Arnaldo
> > > >
> > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > > > >
> > > > > > btf_loader.c | 2 +-
> > > > > > ctf_loader.c | 2 +-
> > > > > > dwarf_loader.c | 4 ++--
> > > > > > dwarves.h | 5 +++--
> > > > > > pahole.c | 3 ++-
> > > > > > pdwtags.c | 3 ++-
> > > > > > pfunct.c | 4 +++-
> > > > > > 7 files changed, 14 insertions(+), 9 deletions(-)
> > > > > >
> > > > > > diff --git a/btf_loader.c b/btf_loader.c
> > > > > > index 7a5b16ff393e..b61cadd55127 100644
> > > > > > --- a/btf_loader.c
> > > > > > +++ b/btf_loader.c
> > > > > > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > > > > > * The app stole this cu, possibly deleting it,
> > > > > > * so forget about it
> > > > > > */
> > > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > > return 0;
> > > > > >
> > > > > > cus__add(cus, cu);
> > > > > > diff --git a/ctf_loader.c b/ctf_loader.c
> > > > > > index 7c34739afdce..de6d4dbfce97 100644
> > > > > > --- a/ctf_loader.c
> > > > > > +++ b/ctf_loader.c
> > > > > > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > > > > > * The app stole this cu, possibly deleting it,
> > > > > > * so forget about it
> > > > > > */
> > > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > > return 0;
> > > > > >
> > > > > > cus__add(cus, cu);
> > > > > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > > > > index e30b03c1c541..bf9ea3765419 100644
> > > > > > --- a/dwarf_loader.c
> > > > > > +++ b/dwarf_loader.c
> > > > > > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > > > > > {
> > > > > > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > > > > > if (conf && conf->steal) {
> > > > > > - return conf->steal(cu, conf);
> > > > > > + return conf->steal(cu, conf, NULL);
> > > > > > }
> > > > > > return LSK__KEEPIT;
> > > > > > }
> > > > > > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > > > > > goto out_abort;
> > > > > > }
> > > > > >
> > > > > > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > > > > > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > > > > > goto out_abort;
> > > > > >
> > > > > > return (void *)DWARF_CB_OK;
> > > > > > diff --git a/dwarves.h b/dwarves.h
> > > > > > index 52d162d67456..9a8e4de8843a 100644
> > > > > > --- a/dwarves.h
> > > > > > +++ b/dwarves.h
> > > > > > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > > > > > */
> > > > > > struct conf_load {
> > > > > > enum load_steal_kind (*steal)(struct cu *cu,
> > > > > > - struct conf_load *conf);
> > > > > > - int (*thread_exit)(void);
> > > > > > + struct conf_load *conf,
> > > > > > + void *thr_data);
> > > > > > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > > > > > void *cookie;
> > > > > > char *format_path;
> > > > > > int nr_jobs;
> > > > > > diff --git a/pahole.c b/pahole.c
> > > > > > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > > > > > --- a/pahole.c
> > > > > > +++ b/pahole.c
> > > > > > @@ -2799,7 +2799,8 @@ out:
> > > > > > static struct type_instance *header;
> > > > > >
> > > > > > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > > > > > - struct conf_load *conf_load)
> > > > > > + struct conf_load *conf_load,
> > > > > > + void *thr_data)
> > > > > > {
> > > > > > int ret = LSK__DELETE;
> > > > > >
> > > > > > diff --git a/pdwtags.c b/pdwtags.c
> > > > > > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > > > > > --- a/pdwtags.c
> > > > > > +++ b/pdwtags.c
> > > > > > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > > > > > }
> > > > > >
> > > > > > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > > > > > - struct conf_load *conf_load __maybe_unused)
> > > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > > + void *thr_data __maybe_unused)
> > > > > > {
> > > > > > cu__emit_tags(cu);
> > > > > > return LSK__DELETE;
> > > > > > diff --git a/pfunct.c b/pfunct.c
> > > > > > index 5485622e639b..314915b774f4 100644
> > > > > > --- a/pfunct.c
> > > > > > +++ b/pfunct.c
> > > > > > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > > > > > return EXIT_SUCCESS;
> > > > > > }
> > > > > >
> > > > > > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > > > > > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > > + void *thr_data __maybe_unused)
> > > > > > {
> > > > > >
> > > > > > if (function_name) {
> > > > > > --
> > > > > > 2.30.2
> > > > > >
> > > >
> > > > --
> > > >
> > > > - Arnaldo
> >
> > --
> >
> > - Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-10 0:14 ` Andrii Nakryiko
2022-03-10 0:18 ` Andrii Nakryiko
@ 2022-03-14 15:38 ` Arnaldo Carvalho de Melo
2022-03-14 15:43 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-14 15:38 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Arnaldo Carvalho de Melo, Kui-Feng Lee, dwarves,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf
Em Wed, Mar 09, 2022 at 04:14:40PM -0800, Andrii Nakryiko escreveu:
> On Wed, Mar 9, 2022 at 11:24 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Tue, Mar 08, 2022 at 03:45:03PM -0800, Andrii Nakryiko escreveu:
> > > On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko escreveu:
> > > > > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee <kuifeng@fb.com> wrote:
> > > > > >
> > > > > > Add arguments to steal and thread_exit callbacks of conf_load to
> > > > > > receive per-thread data.
> > > > > >
> > > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > > > ---
> > > > >
> > > > > Please carry over acks you got on previous revisions, unless you did
> > > > > some drastic changes to already acked patches.
> > > >
> > > > Yes, please do so.
> > > >
> > > > I'll collect them this time, no need to resend.
> > > >
> > >
> > > Hey, Arnaldo!
> > >
> > > Any idea when these patches will make it into master branch? I see
> > > they are in tmp.master right now.
> >
> > I did some minor fixups to the cset comment and to the code in the
> > 'pahole --compile' new feature at the head of it and pushed all up,
> > please check.
> >
>
> I did check locally with latest pahole master, and it seems like
> something is wrong with generated BTF. I get three selftests failure
> if I use latest pahole compiled from master.
>
> Kui-Feng, please take a look when you get a chance. Arnaldo, please
> hold off from releasing a new version for now.
Sure, will wait.
Also will try to test/fix this as time permits.
- Arnaldo
>
> > - Arnaldo
> >
> > > > - Arnaldo
> > > >
> > > > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> > > > >
> > > > > > btf_loader.c | 2 +-
> > > > > > ctf_loader.c | 2 +-
> > > > > > dwarf_loader.c | 4 ++--
> > > > > > dwarves.h | 5 +++--
> > > > > > pahole.c | 3 ++-
> > > > > > pdwtags.c | 3 ++-
> > > > > > pfunct.c | 4 +++-
> > > > > > 7 files changed, 14 insertions(+), 9 deletions(-)
> > > > > >
> > > > > > diff --git a/btf_loader.c b/btf_loader.c
> > > > > > index 7a5b16ff393e..b61cadd55127 100644
> > > > > > --- a/btf_loader.c
> > > > > > +++ b/btf_loader.c
> > > > > > @@ -624,7 +624,7 @@ static int cus__load_btf(struct cus *cus, struct conf_load *conf, const char *fi
> > > > > > * The app stole this cu, possibly deleting it,
> > > > > > * so forget about it
> > > > > > */
> > > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > > return 0;
> > > > > >
> > > > > > cus__add(cus, cu);
> > > > > > diff --git a/ctf_loader.c b/ctf_loader.c
> > > > > > index 7c34739afdce..de6d4dbfce97 100644
> > > > > > --- a/ctf_loader.c
> > > > > > +++ b/ctf_loader.c
> > > > > > @@ -722,7 +722,7 @@ int ctf__load_file(struct cus *cus, struct conf_load *conf,
> > > > > > * The app stole this cu, possibly deleting it,
> > > > > > * so forget about it
> > > > > > */
> > > > > > - if (conf && conf->steal && conf->steal(cu, conf))
> > > > > > + if (conf && conf->steal && conf->steal(cu, conf, NULL))
> > > > > > return 0;
> > > > > >
> > > > > > cus__add(cus, cu);
> > > > > > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > > > > > index e30b03c1c541..bf9ea3765419 100644
> > > > > > --- a/dwarf_loader.c
> > > > > > +++ b/dwarf_loader.c
> > > > > > @@ -2686,7 +2686,7 @@ static int cu__finalize(struct cu *cu, struct conf_load *conf)
> > > > > > {
> > > > > > cu__for_all_tags(cu, class_member__cache_byte_size, conf);
> > > > > > if (conf && conf->steal) {
> > > > > > - return conf->steal(cu, conf);
> > > > > > + return conf->steal(cu, conf, NULL);
> > > > > > }
> > > > > > return LSK__KEEPIT;
> > > > > > }
> > > > > > @@ -2930,7 +2930,7 @@ static void *dwarf_cus__process_cu_thread(void *arg)
> > > > > > goto out_abort;
> > > > > > }
> > > > > >
> > > > > > - if (dcus->conf->thread_exit && dcus->conf->thread_exit() != 0)
> > > > > > + if (dcus->conf->thread_exit && dcus->conf->thread_exit(dcus->conf, NULL) != 0)
> > > > > > goto out_abort;
> > > > > >
> > > > > > return (void *)DWARF_CB_OK;
> > > > > > diff --git a/dwarves.h b/dwarves.h
> > > > > > index 52d162d67456..9a8e4de8843a 100644
> > > > > > --- a/dwarves.h
> > > > > > +++ b/dwarves.h
> > > > > > @@ -48,8 +48,9 @@ struct conf_fprintf;
> > > > > > */
> > > > > > struct conf_load {
> > > > > > enum load_steal_kind (*steal)(struct cu *cu,
> > > > > > - struct conf_load *conf);
> > > > > > - int (*thread_exit)(void);
> > > > > > + struct conf_load *conf,
> > > > > > + void *thr_data);
> > > > > > + int (*thread_exit)(struct conf_load *conf, void *thr_data);
> > > > > > void *cookie;
> > > > > > char *format_path;
> > > > > > int nr_jobs;
> > > > > > diff --git a/pahole.c b/pahole.c
> > > > > > index f3a51cb2fe74..f3eeaaca4cdf 100644
> > > > > > --- a/pahole.c
> > > > > > +++ b/pahole.c
> > > > > > @@ -2799,7 +2799,8 @@ out:
> > > > > > static struct type_instance *header;
> > > > > >
> > > > > > static enum load_steal_kind pahole_stealer(struct cu *cu,
> > > > > > - struct conf_load *conf_load)
> > > > > > + struct conf_load *conf_load,
> > > > > > + void *thr_data)
> > > > > > {
> > > > > > int ret = LSK__DELETE;
> > > > > >
> > > > > > diff --git a/pdwtags.c b/pdwtags.c
> > > > > > index 2b5ba1bf6745..8b1d6f1c96cb 100644
> > > > > > --- a/pdwtags.c
> > > > > > +++ b/pdwtags.c
> > > > > > @@ -72,7 +72,8 @@ static int cu__emit_tags(struct cu *cu)
> > > > > > }
> > > > > >
> > > > > > static enum load_steal_kind pdwtags_stealer(struct cu *cu,
> > > > > > - struct conf_load *conf_load __maybe_unused)
> > > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > > + void *thr_data __maybe_unused)
> > > > > > {
> > > > > > cu__emit_tags(cu);
> > > > > > return LSK__DELETE;
> > > > > > diff --git a/pfunct.c b/pfunct.c
> > > > > > index 5485622e639b..314915b774f4 100644
> > > > > > --- a/pfunct.c
> > > > > > +++ b/pfunct.c
> > > > > > @@ -489,7 +489,9 @@ int elf_symtabs__show(char *filenames[])
> > > > > > return EXIT_SUCCESS;
> > > > > > }
> > > > > >
> > > > > > -static enum load_steal_kind pfunct_stealer(struct cu *cu, struct conf_load *conf_load __maybe_unused)
> > > > > > +static enum load_steal_kind pfunct_stealer(struct cu *cu,
> > > > > > + struct conf_load *conf_load __maybe_unused,
> > > > > > + void *thr_data __maybe_unused)
> > > > > > {
> > > > > >
> > > > > > if (function_name) {
> > > > > > --
> > > > > > 2.30.2
> > > > > >
> > > >
> > > > --
> > > >
> > > > - Arnaldo
> >
> > --
> >
> > - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-14 15:38 ` Arnaldo Carvalho de Melo
@ 2022-03-14 15:43 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-14 15:43 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Kui-Feng Lee, dwarves, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, bpf
Em Mon, Mar 14, 2022 at 12:38:59PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 09, 2022 at 04:14:40PM -0800, Andrii Nakryiko escreveu:
> > I did check locally with latest pahole master, and it seems like
> > something is wrong with generated BTF. I get three selftests failure
> > if I use latest pahole compiled from master.
> > Kui-Feng, please take a look when you get a chance. Arnaldo, please
> > hold off from releasing a new version for now.
> Sure, will wait.
> Also will try to test/fix this as time permits.
The tests at
https://github.com/libbpf/libbpf/actions/workflows/pahole.yml are all
happy with what we have, so maybe a new test is needed for checking
these three selftests into what is being tested there?
And just so that we're all on the same page, this is the current status
in the various branches and git servers:
⬢[acme@toolbox pahole]$ git log --oneline -10
65d7273668ded59b (HEAD -> master, quaco/master, quaco/HEAD, github/tmp.master, github/next, github/master, acme/tmp.master, acme/next, acme/master) pahole: Introduce --compile to produce a compilable output
4d004e2314f3252e core: Ditch 'dwarves__active_loader' extern declaration, it was nuked
4f332dbfd02072e4 emit: Notice type shadowing, i.e. multiple types with the same name (enum, struct, union, etc)
0a82f74ce25a5904 core: Make type->packed_attributes_inferred a one bit member
fac821246c582299 core: type->declaration is just one bit, make it a bitfield member
742f04f89da03665 emit: Search for data structures using its type in addition to its name
32cc1481721c4b11 fprintf: Consider enumerations without members as forward declarations
6afc296eeb180e25 emit: Fix printing typedef of nameless struct/union
49a2dd657728675b fprintf: Check if conf->conf_fprintf is not NULL when resolving cacheline_size
46cec35ff0411e0f fprintf: Fix division by zero for uninitialized conf_fprintf->cacheline_size field
⬢[acme@toolbox pahole]$
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-10 0:18 ` Andrii Nakryiko
@ 2022-03-21 19:06 ` Kui-Feng Lee
2022-03-21 21:08 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 25+ messages in thread
From: Kui-Feng Lee @ 2022-03-21 19:06 UTC (permalink / raw)
To: acme, andrii.nakryiko; +Cc: daniel, ast, arnaldo.melo, andrii, dwarves, bpf
On Wed, 2022-03-09 at 16:18 -0800, Andrii Nakryiko wrote:
> On Wed, Mar 9, 2022 at 4:14 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Mar 9, 2022 at 11:24 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Tue, Mar 08, 2022 at 03:45:03PM -0800, Andrii Nakryiko
> > > escreveu:
> > > > On Thu, Jan 27, 2022 at 11:22 AM Arnaldo Carvalho de Melo
> > > > <arnaldo.melo@gmail.com> wrote:
> > > > >
> > > > > Em Wed, Jan 26, 2022 at 11:55:25AM -0800, Andrii Nakryiko
> > > > > escreveu:
> > > > > > On Wed, Jan 26, 2022 at 11:20 AM Kui-Feng Lee
> > > > > > <kuifeng@fb.com> wrote:
> > > > > > >
> > > > > > > Add arguments to steal and thread_exit callbacks of
> > > > > > > conf_load to
> > > > > > > receive per-thread data.
> > > > > > >
> > > > > > > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > > > > > > ---
> > > > > >
> > > > > > Please carry over acks you got on previous revisions,
> > > > > > unless you did
> > > > > > some drastic changes to already acked patches.
> > > > >
> > > > > Yes, please do so.
> > > > >
> > > > > I'll collect them this time, no need to resend.
> > > > >
> > > >
> > > > Hey, Arnaldo!
> > > >
> > > > Any idea when these patches will make it into master branch? I
> > > > see
> > > > they are in tmp.master right now.
> > >
> > > I did some minor fixups to the cset comment and to the code in
> > > the
> > > 'pahole --compile' new feature at the head of it and pushed all
> > > up,
> > > please check.
> > >
> >
> > I did check locally with latest pahole master, and it seems like
> > something is wrong with generated BTF. I get three selftests
> > failure
> > if I use latest pahole compiled from master.
> >
> > Kui-Feng, please take a look when you get a chance. Arnaldo, please
> > hold off from releasing a new version for now.
I just figure out the root cuase.
It caused by missing info from percpu_secinfo when collecting data from
threads. The encoder stores it separatedly from struct btf, and we
have separated different encoders for threads. They are not merged
together. I will fixed it ASAP.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads.
2022-03-21 19:06 ` Kui-Feng Lee
@ 2022-03-21 21:08 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-21 21:08 UTC (permalink / raw)
To: Kui-Feng Lee
Cc: Andrii Nakryiko, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, dwarves, bpf
Em Mon, Mar 21, 2022 at 07:06:06PM +0000, Kui-Feng Lee escreveu:
> On Wed, 2022-03-09 at 16:18 -0800, Andrii Nakryiko wrote:
> > On Wed, Mar 9, 2022 at 4:14 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > I did check locally with latest pahole master, and it seems like
> > > something is wrong with generated BTF. I get three selftests
> > > failure
> > > if I use latest pahole compiled from master.
> > > Kui-Feng, please take a look when you get a chance. Arnaldo, please
> > > hold off from releasing a new version for now.
> I just figure out the root cuase.
> It caused by missing info from percpu_secinfo when collecting data from
> threads. The encoder stores it separatedly from struct btf, and we
> have separated different encoders for threads. They are not merged
> together. I will fixed it ASAP.
Cool, looking forward to the fix.
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2022-03-21 21:08 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 19:20 [PATCH dwarves v4 0/4] Parallelize BTF type info generating of pahole Kui-Feng Lee
2022-01-26 19:20 ` [PATCH dwarves v4 1/4] dwarf_loader: Receive per-thread data on worker threads Kui-Feng Lee
2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 20:14 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
2022-03-08 23:45 ` Andrii Nakryiko
2022-03-09 19:24 ` Arnaldo Carvalho de Melo
2022-03-10 0:14 ` Andrii Nakryiko
2022-03-10 0:18 ` Andrii Nakryiko
2022-03-21 19:06 ` Kui-Feng Lee
2022-03-21 21:08 ` Arnaldo Carvalho de Melo
2022-03-14 15:38 ` Arnaldo Carvalho de Melo
2022-03-14 15:43 ` Arnaldo Carvalho de Melo
2022-01-26 19:20 ` [PATCH dwarves v4 2/4] dwarf_loader: Prepare and pass per-thread data to " Kui-Feng Lee
2022-01-26 19:55 ` Andrii Nakryiko
2022-01-26 19:20 ` [PATCH dwarves v4 3/4] pahole: Use per-thread btf instances to avoid mutex locking Kui-Feng Lee
2022-01-26 19:58 ` Andrii Nakryiko
2022-01-26 20:57 ` Kui-Feng Lee
2022-01-27 10:05 ` Arnaldo Carvalho de Melo
2022-01-27 20:12 ` Arnaldo Carvalho de Melo
2022-01-28 19:50 ` Arnaldo Carvalho de Melo
2022-02-01 6:56 ` Andrii Nakryiko
2022-02-02 0:16 ` Arnaldo Carvalho de Melo
2022-01-26 19:20 ` [PATCH dwarves v4 4/4] libbpf: Update libbpf to a new revision Kui-Feng Lee
2022-01-26 19:59 ` Andrii Nakryiko
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).