On Mon, 2024-10-14 at 13:08 -0700, Eduard Zingerman wrote: > On Mon, 2024-10-14 at 19:39 +0000, Ihor Solodrai wrote: > > [...] > > > Ok, I'll try making the explanation better. > > > > Just so it's clear, in this patchset there are no changes to > > btf_encoder__save_func() at all. > > [...] > > > I remember we discussed off-list simple accumulation of elf_function > > structs, and removing merge logic from btf_encoder__save_func(). I am > > hesitant to switch to that, because this would mean that multiple > > elf_function objects for the same encoder might be stored in > > elf_functions table. This in turn would require more changes. For > > example, how btf_encoder__find_function() should behave then? > > Then I don't understand how elf_function objects are protected against > concurrent update, btf_encoder__save_func(): > - mutates elf_function instance; > - is called from btf_encoder__encode_cu(), called from pahole_stealer(), > executed concurrently; > > Here is a fragment of the btf_encoder__save_func(): > > existing->optimized_parms |= state.optimized_parms; > existing->unexpected_reg |= state.unexpected_reg; > if (!existing->unexpected_reg && > !funcs__match(encoder, func, encoder->btf, &state, > encoder->btf, existing)) > existing->inconsistent_proto = 1; > > If concurrent access to elf_function instance is allowed, > '|=' operations need to be atomic and funcs_match() needs some mutex to > prevent BTF update (as you point out in the commit message). > What do I miss? Ok, I forgot about the implementation of the find_elf_function_in_list(). Please ignore my previous comment and sorry for the noise.
