* [PATCH dwarves v4 1/3] btf_encoder: Remove encoder pointer from btf_encoder_func_state
2025-11-06 1:28 [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Ihor Solodrai
@ 2025-11-06 1:28 ` Ihor Solodrai
2025-11-06 1:28 ` [PATCH dwarves v4 2/3] btf_encoder: Refactor btf_encoder__add_func_proto Ihor Solodrai
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Ihor Solodrai @ 2025-11-06 1:28 UTC (permalink / raw)
To: dwarves, alan.maguire, acme, eddyz87; +Cc: bpf, andrii, ast, kernel-team
Since multithreading was moved to dwarf_loader [1], pahole maintains a
single global btf_encoder instance. However parts of btf_encoder.c
exist to manage mulitple encoders, which is not necessary anymore.
This patch removes encoder pointer from struct btf_encoder_func_state.
We create a state for every encountered instance of a function in
DWARF, and currently they carry around the encoder unnecessarily.
[1] https://lore.kernel.org/all/20250109185950.653110-9-ihor.solodrai@pm.me/
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
Tested-by: Eduard Zingerman <eddyz87@gmail.com>
---
btf_encoder.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index 03bc3c7..a6fdc58 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -79,7 +79,6 @@ struct btf_encoder_func_annot {
/* state used to do later encoding of saved functions */
struct btf_encoder_func_state {
- struct btf_encoder *encoder;
struct elf_function *elf;
uint32_t type_id_off;
uint16_t nr_parms;
@@ -819,7 +818,7 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
struct btf_encoder_func_state *state)
{
const struct btf_type *t;
- struct btf *btf;
+ struct btf *btf = encoder->btf;
struct parameter *param;
uint16_t nr_params, param_idx;
int32_t id, type_id;
@@ -834,12 +833,9 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f
/* add btf_type for func_proto */
if (ftype) {
- btf = encoder->btf;
nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
} else if (state) {
- encoder = state->encoder;
- btf = state->encoder->btf;
nr_params = state->nr_parms;
type_id = state->ret_type_id;
} else {
@@ -1123,13 +1119,12 @@ static bool types__match(struct btf_encoder *encoder,
return false;
}
-static bool funcs__match(struct btf_encoder_func_state *s1,
+static bool funcs__match(struct btf_encoder *encoder,
+ struct btf_encoder_func_state *s1,
struct btf_encoder_func_state *s2)
{
- struct btf_encoder *encoder = s1->encoder;
struct elf_function *func = s1->elf;
- struct btf *btf1 = s1->encoder->btf;
- struct btf *btf2 = s2->encoder->btf;
+ struct btf *btf = encoder->btf;
uint8_t i;
if (s1->nr_parms != s2->nr_parms) {
@@ -1138,7 +1133,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
s1->nr_parms, s2->nr_parms);
return false;
}
- if (!types__match(encoder, btf1, s1->ret_type_id, btf2, s2->ret_type_id)) {
+ if (!types__match(encoder, btf, s1->ret_type_id, btf, s2->ret_type_id)) {
btf_encoder__log_func_skip(encoder, func, "return type mismatch\n");
return false;
}
@@ -1146,11 +1141,11 @@ static bool funcs__match(struct btf_encoder_func_state *s1,
return true;
for (i = 0; i < s1->nr_parms; i++) {
- if (!types__match(encoder, btf1, s1->parms[i].type_id,
- btf2, s2->parms[i].type_id)) {
+ if (!types__match(encoder, btf, s1->parms[i].type_id,
+ btf, s2->parms[i].type_id)) {
if (encoder->verbose) {
- const char *p1 = btf__name_by_offset(btf1, s1->parms[i].name_off);
- const char *p2 = btf__name_by_offset(btf2, s2->parms[i].name_off);
+ const char *p1 = btf__name_by_offset(btf, s1->parms[i].name_off);
+ const char *p2 = btf__name_by_offset(btf, s2->parms[i].name_off);
btf_encoder__log_func_skip(encoder, func,
"param type mismatch for param#%d %s %s %s\n",
@@ -1244,7 +1239,6 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
if (!state)
return -ENOMEM;
- state->encoder = encoder;
state->elf = func;
state->nr_parms = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
state->ret_type_id = ftype->tag.type == 0 ? 0 : encoder->type_id_off + ftype->tag.type;
@@ -1410,7 +1404,9 @@ static int saved_functions_cmp(const void *_a, const void *_b)
return elf_function__name_cmp(a->elf, b->elf);
}
-static int saved_functions_combine(struct btf_encoder_func_state *a, struct btf_encoder_func_state *b)
+static int saved_functions_combine(struct btf_encoder *encoder,
+ struct btf_encoder_func_state *a,
+ struct btf_encoder_func_state *b)
{
uint8_t optimized, unexpected, inconsistent, uncertain_parm_loc;
@@ -1421,7 +1417,7 @@ static int saved_functions_combine(struct btf_encoder_func_state *a, struct btf_
unexpected = a->unexpected_reg | b->unexpected_reg;
inconsistent = a->inconsistent_proto | b->inconsistent_proto;
uncertain_parm_loc = a->uncertain_parm_loc | b->uncertain_parm_loc;
- if (!unexpected && !inconsistent && !funcs__match(a, b))
+ if (!unexpected && !inconsistent && !funcs__match(encoder, a, b))
inconsistent = 1;
a->optimized_parms = b->optimized_parms = optimized;
a->unexpected_reg = b->unexpected_reg = unexpected;
@@ -1471,7 +1467,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
*/
j = i + 1;
- while (j < nr_saved_fns && saved_functions_combine(&saved_fns[i], &saved_fns[j]) == 0)
+ while (j < nr_saved_fns && saved_functions_combine(encoder, &saved_fns[i], &saved_fns[j]) == 0)
j++;
/* do not exclude functions with optimized-out parameters; they
@@ -1488,7 +1484,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
0, 0);
if (add_to_btf) {
- err = btf_encoder__add_func(state->encoder, state);
+ err = btf_encoder__add_func(encoder, state);
if (err < 0)
goto out;
}
--
2.51.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH dwarves v4 2/3] btf_encoder: Refactor btf_encoder__add_func_proto
2025-11-06 1:28 [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Ihor Solodrai
2025-11-06 1:28 ` [PATCH dwarves v4 1/3] btf_encoder: Remove encoder pointer from btf_encoder_func_state Ihor Solodrai
@ 2025-11-06 1:28 ` Ihor Solodrai
2025-11-06 1:28 ` [PATCH dwarves v4 3/3] btf_encoder: Factor out BPF kfunc emission Ihor Solodrai
2025-11-13 16:36 ` [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Alan Maguire
3 siblings, 0 replies; 12+ messages in thread
From: Ihor Solodrai @ 2025-11-06 1:28 UTC (permalink / raw)
To: dwarves, alan.maguire, acme, eddyz87; +Cc: bpf, andrii, ast, kernel-team
btf_encoder__add_func_proto() essentially implements two independent
code paths depending on input arguments: one for struct ftype and the
other for struct btf_encoder_func_state.
Split btf_encoder__add_func_proto() into two variants:
* btf_encoder__add_func_proto_for_ftype()
* btf_encoder__add_func_proto_for_state()
And factor out common btf_encoder__emit_func_proto() subroutine.
No functional changes.
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
Tested-by: Eduard Zingerman <eddyz87@gmail.com>
---
btf_encoder.c | 135 +++++++++++++++++++++++++++++---------------------
1 file changed, 79 insertions(+), 56 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index a6fdc58..bdda7d0 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -814,79 +814,102 @@ static inline bool is_kfunc_state(struct btf_encoder_func_state *state)
return state && state->elf && state->elf->kfunc;
}
-static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
- struct btf_encoder_func_state *state)
+static int32_t btf_encoder__emit_func_proto(struct btf_encoder *encoder,
+ uint32_t type_id,
+ uint16_t nr_params)
{
const struct btf_type *t;
- struct btf *btf = encoder->btf;
- struct parameter *param;
+ uint32_t ret;
+
+ ret = btf__add_func_proto(encoder->btf, type_id);
+ if (ret > 0) {
+ t = btf__type_by_id(encoder->btf, ret);
+ btf_encoder__log_type(encoder, t, false, false,
+ "return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
+ } else {
+ btf__log_err(encoder->btf, BTF_KIND_FUNC_PROTO, NULL, true, ret,
+ "return=%u vlen=%u Error emitting BTF type",
+ type_id, nr_params);
+ }
+
+ return ret;
+}
+
+static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder,
+ struct ftype *ftype)
+{
uint16_t nr_params, param_idx;
+ struct parameter *param;
int32_t id, type_id;
- char tmp_name[KSYM_NAME_LEN];
const char *name;
- assert(ftype != NULL || state != NULL);
-
- if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
- if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
- return -1;
+ assert(ftype != NULL);
/* add btf_type for func_proto */
- if (ftype) {
- nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
- type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
- } else if (state) {
- nr_params = state->nr_parms;
- type_id = state->ret_type_id;
- } else {
- return 0;
- }
+ nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
+ type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
- id = btf__add_func_proto(btf, type_id);
- if (id > 0) {
- t = btf__type_by_id(btf, id);
- btf_encoder__log_type(encoder, t, false, false, "return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
- } else {
- btf__log_err(btf, BTF_KIND_FUNC_PROTO, NULL, true, id,
- "return=%u vlen=%u Error emitting BTF type",
- type_id, nr_params);
+ id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+ if (id < 0)
return id;
- }
/* add parameters */
param_idx = 0;
- if (ftype) {
- ftype__for_each_parameter(ftype, param) {
- const char *name = parameter__name(param);
-
- type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
- ++param_idx;
- if (btf_encoder__add_func_param(encoder, name, type_id,
- param_idx == nr_params))
- return -1;
- }
+ ftype__for_each_parameter(ftype, param) {
+ name = parameter__name(param);
+ type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
++param_idx;
- if (ftype->unspec_parms)
- if (btf_encoder__add_func_param(encoder, NULL, 0,
- param_idx == nr_params))
- return -1;
- } else {
- for (param_idx = 0; param_idx < nr_params; param_idx++) {
- struct btf_encoder_func_parm *p = &state->parms[param_idx];
+ if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params))
+ return -1;
+ }
- name = btf__name_by_offset(btf, p->name_off);
+ ++param_idx;
+ if (ftype->unspec_parms)
+ if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
+ return -1;
- /* adding BTF data may result in a move of the
- * name string memory, so make a temporary copy.
- */
- strncpy(tmp_name, name, sizeof(tmp_name) - 1);
+ return id;
+}
- if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id,
- param_idx == nr_params))
- return -1;
- }
+static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder,
+ struct btf_encoder_func_state *state)
+{
+ const struct btf *btf = encoder->btf;
+ struct btf_encoder_func_parm *p;
+ uint16_t nr_params, param_idx;
+ char tmp_name[KSYM_NAME_LEN];
+ int32_t id, type_id;
+ const char *name;
+ bool is_last;
+
+ /* Beware: btf__add_bpf_arena_type_tags may change some members of the state */
+ if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
+ if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+ return -1;
+
+ type_id = state->ret_type_id;
+ nr_params = state->nr_parms;
+
+ id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+ if (id < 0)
+ return id;
+
+ /* add parameters */
+ for (param_idx = 0; param_idx < nr_params; param_idx++) {
+ p = &state->parms[param_idx];
+ name = btf__name_by_offset(btf, p->name_off);
+ is_last = param_idx == nr_params;
+
+ /* adding BTF data may result in a move of the
+ * name string memory, so make a temporary copy.
+ */
+ strncpy(tmp_name, name, sizeof(tmp_name) - 1);
+
+ if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id, is_last))
+ return -1;
}
+
return id;
}
@@ -1343,7 +1366,7 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
uint16_t idx;
int err;
- btf_fnproto_id = btf_encoder__add_func_proto(encoder, NULL, state);
+ btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state);
name = func->name;
if (btf_fnproto_id >= 0)
btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id,
@@ -1682,7 +1705,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
case DW_TAG_enumeration_type:
return btf_encoder__add_enum_type(encoder, tag, conf_load);
case DW_TAG_subroutine_type:
- return btf_encoder__add_func_proto(encoder, tag__ftype(tag), NULL);
+ return btf_encoder__add_func_proto_for_ftype(encoder, tag__ftype(tag));
case DW_TAG_unspecified_type:
/* Just don't encode this for now, converting anything with this type to void (0) instead.
*
--
2.51.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH dwarves v4 3/3] btf_encoder: Factor out BPF kfunc emission
2025-11-06 1:28 [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Ihor Solodrai
2025-11-06 1:28 ` [PATCH dwarves v4 1/3] btf_encoder: Remove encoder pointer from btf_encoder_func_state Ihor Solodrai
2025-11-06 1:28 ` [PATCH dwarves v4 2/3] btf_encoder: Refactor btf_encoder__add_func_proto Ihor Solodrai
@ 2025-11-06 1:28 ` Ihor Solodrai
2025-11-13 16:36 ` [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Alan Maguire
3 siblings, 0 replies; 12+ messages in thread
From: Ihor Solodrai @ 2025-11-06 1:28 UTC (permalink / raw)
To: dwarves, alan.maguire, acme, eddyz87; +Cc: bpf, andrii, ast, kernel-team
Generating BTF for BPF kernel functions requires special
handling. Consolidate this behavior into btf_encoder__add_bpf_kfunc(),
which uses simplified btf_encoder_add_func().
No functional changes.
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
Tested-by: Eduard Zingerman <eddyz87@gmail.com>
---
btf_encoder.c | 51 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index bdda7d0..b37ee7f 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -773,6 +773,7 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state
return id;
}
+/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */
static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state)
{
uint32_t flags = state->elf->kfunc_flags;
@@ -883,11 +884,6 @@ static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder
const char *name;
bool is_last;
- /* Beware: btf__add_bpf_arena_type_tags may change some members of the state */
- if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
- if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
- return -1;
-
type_id = state->ret_type_id;
nr_params = state->nr_parms;
@@ -1357,14 +1353,13 @@ static int btf__tag_kfunc(struct btf *btf, struct elf_function *kfunc, __u32 btf
static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
struct btf_encoder_func_state *state)
{
- struct elf_function *func = state->elf;
int btf_fnproto_id, btf_fn_id, tag_type_id = 0;
+ struct elf_function *func = state->elf;
+ char tmp_value[KSYM_NAME_LEN];
int16_t component_idx = -1;
- const char *name;
const char *value;
- char tmp_value[KSYM_NAME_LEN];
+ const char *name;
uint16_t idx;
- int err;
btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state);
name = func->name;
@@ -1377,15 +1372,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
return -1;
}
- if (func->kfunc && encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
- err = btf__tag_kfunc(encoder->btf, func, btf_fn_id);
- if (err < 0)
- return err;
- }
-
- if (state->nr_annots == 0)
- return 0;
-
for (idx = 0; idx < state->nr_annots; idx++) {
struct btf_encoder_func_annot *a = &state->annots[idx];
@@ -1408,6 +1394,30 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
return -1;
}
+ return btf_fn_id;
+}
+
+static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
+ struct btf_encoder_func_state *state)
+{
+ int btf_fn_id, err;
+
+ if (encoder->tag_kfuncs && encoder->encode_attributes) {
+ err = btf__add_bpf_arena_type_tags(encoder->btf, state);
+ if (err < 0)
+ return err;
+ }
+
+ btf_fn_id = btf_encoder__add_func(encoder, state);
+ if (btf_fn_id < 0)
+ return -1;
+
+ if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
+ err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id);
+ if (err < 0)
+ return err;
+ }
+
return 0;
}
@@ -1507,7 +1517,10 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
0, 0);
if (add_to_btf) {
- err = btf_encoder__add_func(encoder, state);
+ if (is_kfunc_state(state))
+ err = btf_encoder__add_bpf_kfunc(encoder, state);
+ else
+ err = btf_encoder__add_func(encoder, state);
if (err < 0)
goto out;
}
--
2.51.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-06 1:28 [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Ihor Solodrai
` (2 preceding siblings ...)
2025-11-06 1:28 ` [PATCH dwarves v4 3/3] btf_encoder: Factor out BPF kfunc emission Ihor Solodrai
@ 2025-11-13 16:36 ` Alan Maguire
2025-11-13 17:20 ` Alexei Starovoitov
3 siblings, 1 reply; 12+ messages in thread
From: Alan Maguire @ 2025-11-13 16:36 UTC (permalink / raw)
To: Ihor Solodrai, dwarves, acme, eddyz87; +Cc: bpf, andrii, ast, kernel-team
On 06/11/2025 01:28, Ihor Solodrai wrote:
> This series refactors a few functions that handle how BTF functions
> are emitted.
>
> v3->v4: Error handling nit from Eduard
> v2->v3: Add patch removing encoder from btf_encoder_func_state
>
> v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
> v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
> v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
>
series applied to the next branch of
https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
Thank you!
> Ihor Solodrai (3):
> btf_encoder: Remove encoder pointer from btf_encoder_func_state
> btf_encoder: Refactor btf_encoder__add_func_proto
> btf_encoder: Factor out BPF kfunc emission
>
> btf_encoder.c | 206 +++++++++++++++++++++++++++++---------------------
> 1 file changed, 119 insertions(+), 87 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-13 16:36 ` [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs Alan Maguire
@ 2025-11-13 17:20 ` Alexei Starovoitov
2025-11-14 2:10 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 12+ messages in thread
From: Alexei Starovoitov @ 2025-11-13 17:20 UTC (permalink / raw)
To: Alan Maguire
Cc: Ihor Solodrai, dwarves, Arnaldo Carvalho de Melo, Eduard, bpf,
Andrii Nakryiko, Alexei Starovoitov, Kernel Team
On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 06/11/2025 01:28, Ihor Solodrai wrote:
> > This series refactors a few functions that handle how BTF functions
> > are emitted.
> >
> > v3->v4: Error handling nit from Eduard
> > v2->v3: Add patch removing encoder from btf_encoder_func_state
> >
> > v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
> > v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
> > v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
> >
>
> series applied to the next branch of
> https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
Same rant as before...
Can we please keep it normal with all changes going to master ?
This 'next' branch confused people in the past.
I see no value in this 'next' thing.
All development should happen in master and every developer
should base their changes on top of it.
Eventually the release happens out of master too when it's deemed stable.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-13 17:20 ` Alexei Starovoitov
@ 2025-11-14 2:10 ` Arnaldo Carvalho de Melo
2025-11-14 2:14 ` Alexei Starovoitov
2025-11-14 15:40 ` Alan Maguire
0 siblings, 2 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-11-14 2:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alan Maguire, Ihor Solodrai, dwarves, Eduard, bpf,
Andrii Nakryiko, Alexei Starovoitov, Kernel Team
On Thu, Nov 13, 2025 at 09:20:44AM -0800, Alexei Starovoitov wrote:
> On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > On 06/11/2025 01:28, Ihor Solodrai wrote:
> > > This series refactors a few functions that handle how BTF functions
> > > are emitted.
> > >
> > > v3->v4: Error handling nit from Eduard
> > > v2->v3: Add patch removing encoder from btf_encoder_func_state
> > >
> > > v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
> > > v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
> > > v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
> > >
> >
> > series applied to the next branch of
> > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
>
> Same rant as before...
> Can we please keep it normal with all changes going to master ?
> This 'next' branch confused people in the past.
I think the problem before was that it sat there for far too long.
I see value in it staying there for a short period for some eventual
rebase and for some CI thing, to avoid polluting, think of it as some
topic branch on the way to master.
- Arnaldo
> I see no value in this 'next' thing.
> All development should happen in master and every developer
> should base their changes on top of it.
> Eventually the release happens out of master too when it's deemed stable.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-14 2:10 ` Arnaldo Carvalho de Melo
@ 2025-11-14 2:14 ` Alexei Starovoitov
2025-11-14 15:40 ` Alan Maguire
1 sibling, 0 replies; 12+ messages in thread
From: Alexei Starovoitov @ 2025-11-14 2:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Alan Maguire, Ihor Solodrai, dwarves, Eduard, bpf,
Andrii Nakryiko, Alexei Starovoitov, Kernel Team
On Thu, Nov 13, 2025 at 6:10 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Thu, Nov 13, 2025 at 09:20:44AM -0800, Alexei Starovoitov wrote:
> > On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > On 06/11/2025 01:28, Ihor Solodrai wrote:
> > > > This series refactors a few functions that handle how BTF functions
> > > > are emitted.
> > > >
> > > > v3->v4: Error handling nit from Eduard
> > > > v2->v3: Add patch removing encoder from btf_encoder_func_state
> > > >
> > > > v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
> > > > v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
> > > > v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
> > > >
> > >
> > > series applied to the next branch of
> > > https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
> >
> > Same rant as before...
> > Can we please keep it normal with all changes going to master ?
> > This 'next' branch confused people in the past.
>
> I think the problem before was that it sat there for far too long.
>
> I see value in it staying there for a short period for some eventual
> rebase and for some CI thing, to avoid polluting, think of it as some
> topic branch on the way to master.
I'm still missing the value proposition.
Right now Yonghong is also sending pahole patches,
Ihor will start moving things from pahole to resolve_btfid,
Donglin ais lso working on btf sorting,
and finally Alan has large loc* patchset.
Some or all of them will conflict.
What do they suppose to use as a base?
'next' branch? All of them or some?
How will you differentiate and educate developers ?
Really, 'next' branch is not a good idea.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-14 2:10 ` Arnaldo Carvalho de Melo
2025-11-14 2:14 ` Alexei Starovoitov
@ 2025-11-14 15:40 ` Alan Maguire
2025-11-14 16:27 ` Arnaldo Melo
2025-11-14 20:52 ` Arnaldo Carvalho de Melo
1 sibling, 2 replies; 12+ messages in thread
From: Alan Maguire @ 2025-11-14 15:40 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexei Starovoitov
Cc: Ihor Solodrai, dwarves, Eduard, bpf, Andrii Nakryiko,
Alexei Starovoitov, Kernel Team
On 14/11/2025 02:10, Arnaldo Carvalho de Melo wrote:
> On Thu, Nov 13, 2025 at 09:20:44AM -0800, Alexei Starovoitov wrote:
>> On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>
>>> On 06/11/2025 01:28, Ihor Solodrai wrote:
>>>> This series refactors a few functions that handle how BTF functions
>>>> are emitted.
>>>>
>>>> v3->v4: Error handling nit from Eduard
>>>> v2->v3: Add patch removing encoder from btf_encoder_func_state
>>>>
>>>> v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
>>>> v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
>>>> v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
>>>>
>>>
>>> series applied to the next branch of
>>> https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
>>
>> Same rant as before...
>> Can we please keep it normal with all changes going to master ?
>> This 'next' branch confused people in the past.
>
> I think the problem before was that it sat there for far too long.
>
> I see value in it staying there for a short period for some eventual
> rebase and for some CI thing, to avoid polluting, think of it as some
> topic branch on the way to master.
>
Yeah, I think if we can augment CI to cover more we can narrow this
window, aiming for zero as the test coverage improves. The other thing
we should think about maybe is syncing github.com/acmel/dwarves with
pahole.git as many people are pulling from github. Should we discourage
using the github repo, or just find a way to mirror pahole.git
automatically? Thanks!
Alan
> - Arnaldo
>
>> I see no value in this 'next' thing.
>> All development should happen in master and every developer
>> should base their changes on top of it.
>> Eventually the release happens out of master too when it's deemed stable.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-14 15:40 ` Alan Maguire
@ 2025-11-14 16:27 ` Arnaldo Melo
2025-11-14 20:52 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 12+ messages in thread
From: Arnaldo Melo @ 2025-11-14 16:27 UTC (permalink / raw)
To: Alan Maguire, Arnaldo Carvalho de Melo, Alexei Starovoitov
Cc: Ihor Solodrai, dwarves, Eduard, bpf, Andrii Nakryiko,
Alexei Starovoitov, Kernel Team
7
On November 14, 2025 12:40:36 PM GMT-03:00, Alan Maguire <alan.maguire@oracle.com> wrote:
>On 14/11/2025 02:10, Arnaldo Carvalho de Melo wrote:
>> On Thu, Nov 13, 2025 at 09:20:44AM -0800, Alexei Starovoitov wrote:
>>> On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>>
>>>> On 06/11/2025 01:28, Ihor Solodrai wrote:
>>>>> This series refactors a few functions that handle how BTF functions
>>>>> are emitted.
>>>>>
>>>>> v3->v4: Error handling nit from Eduard
>>>>> v2->v3: Add patch removing encoder from btf_encoder_func_state
>>>>>
>>>>> v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
>>>>> v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
>>>>> v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
>>>>>
>>>>
>>>> series applied to the next branch of
>>>> https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
>>>
>>> Same rant as before...
>>> Can we please keep it normal with all changes going to master ?
>>> This 'next' branch confused people in the past.
>>
>> I think the problem before was that it sat there for far too long.
>>
>> I see value in it staying there for a short period for some eventual
>> rebase and for some CI thing, to avoid polluting, think of it as some
>> topic branch on the way to master.
>>
>
>Yeah, I think if we can augment CI to cover more we can narrow this
>window, aiming for zero as the test coverage improves.
I slept over this and think that is we can have the CI test patches directly from the mailing list, doing AI reviewing even, then the next thing can get a good riddance.
The other thing
>we should think about maybe is syncing github.com/acmel/dwarves with
>pahole.git as many people are pulling from github. Should we discourage
>using the github repo, or just find a way to mirror pahole.git
>automatically? Thanks!
I think we should create a GitHub.com/pahole/ and have shared maintainership, will look into that.
Meanwhile, today, I'll check what's in next, test it and move it to master both in kernel.org and GitHub.
- Arnaldo
>
>Alan
>
>> - Arnaldo
>>
>>> I see no value in this 'next' thing.
>>> All development should happen in master and every developer
>>> should base their changes on top of it.
>>> Eventually the release happens out of master too when it's deemed stable.
>>
>
- Arnaldo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-14 15:40 ` Alan Maguire
2025-11-14 16:27 ` Arnaldo Melo
@ 2025-11-14 20:52 ` Arnaldo Carvalho de Melo
2025-11-14 20:54 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-11-14 20:52 UTC (permalink / raw)
To: Alan Maguire
Cc: Alexei Starovoitov, Ihor Solodrai, dwarves, Eduard, bpf,
Andrii Nakryiko, Alexei Starovoitov, Kernel Team
On Fri, Nov 14, 2025 at 03:40:36PM +0000, Alan Maguire wrote:
> On 14/11/2025 02:10, Arnaldo Carvalho de Melo wrote:
> > On Thu, Nov 13, 2025 at 09:20:44AM -0800, Alexei Starovoitov wrote:
> >> On Thu, Nov 13, 2025 at 8:37 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>>
> >>> On 06/11/2025 01:28, Ihor Solodrai wrote:
> >>>> This series refactors a few functions that handle how BTF functions
> >>>> are emitted.
> >>>>
> >>>> v3->v4: Error handling nit from Eduard
> >>>> v2->v3: Add patch removing encoder from btf_encoder_func_state
> >>>> v3: https://lore.kernel.org/dwarves/20251105185926.296539-1-ihor.solodrai@linux.dev/
> >>>> v2: https://lore.kernel.org/dwarves/20251104233532.196287-1-ihor.solodrai@linux.dev/
> >>>> v1: https://lore.kernel.org/dwarves/20251029190249.3323752-2-ihor.solodrai@linux.dev/
> >>> series applied to the next branch of
> >>> https://git.kernel.org/pub/scm/devel/pahole/pahole.git/
> >> Same rant as before...
> >> Can we please keep it normal with all changes going to master ?
> >> This 'next' branch confused people in the past.
> >
> > I think the problem before was that it sat there for far too long.
> >
> > I see value in it staying there for a short period for some eventual
> > rebase and for some CI thing, to avoid polluting, think of it as some
> > topic branch on the way to master.
Somehow my reply via the smartphone didn't seem to have made to the
list...
> Yeah, I think if we can augment CI to cover more we can narrow this
> window, aiming for zero as the test coverage improves.
So the 'next' is an artifact for CI usage, i.e. if we just don't
announce that it was merged, do it for the CI sake and then when it runs
and don't detect any problem we go ahead and merge into master and
announce that it was merged, nothing of this drama would take place.
The best thing going forward I think is to have AI reviewing just like
with the BPF patches, having the patches, once posted to the list by the
contributors, trigger the CI, that amongst other things do the AI
reviewing, if all is ok, maintainers get some nudge that AI bot is ok,
etc and merge it in master, annouce publicly, yadda, yadda.
> The other thing we should think about maybe is syncing
> github.com/acmel/dwarves with pahole.git as many people are pulling
> from github.
> Should we discourage using the github repo, or just find a way to
> mirror pahole.git automatically? Thanks!
I think we should have a https://github.com/pahole/ area that is
administered by the pahole maintainer(s), just like there are:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools.git/
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
For the project (current merge window, next merge window) and my
personal, previously canonical, perf and other kernel work repo:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/
:-)
- Arnaldo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH dwarves v4 0/3] btf_encoder: refactor emission of BTF funcs
2025-11-14 20:52 ` Arnaldo Carvalho de Melo
@ 2025-11-14 20:54 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-11-14 20:54 UTC (permalink / raw)
To: Alan Maguire
Cc: Alexei Starovoitov, Ihor Solodrai, dwarves, Eduard, bpf,
Andrii Nakryiko, Alexei Starovoitov, Kernel Team
On Fri, Nov 14, 2025 at 05:52:41PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Nov 14, 2025 at 03:40:36PM +0000, Alan Maguire wrote:
> > Yeah, I think if we can augment CI to cover more we can narrow this
> > window, aiming for zero as the test coverage improves.
>
> So the 'next' is an artifact for CI usage, i.e. if we just don't
> announce that it was merged, do it for the CI sake and then when it runs
> and don't detect any problem we go ahead and merge into master and
> announce that it was merged, nothing of this drama would take place.
BTW, I looked at the patches, ran tests and merged next into master,
pushed out to master in both kernel.org and github.
- Arnaldo
^ permalink raw reply [flat|nested] 12+ messages in thread