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? > I am inclined to stick with current (v2) approach, but want to make > sure you're aware of this. > > Let me know what you think. [...]
