On Monday, October 14th, 2024 at 1:20 PM, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > [...] > > > > > 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. Yeah. In case it's unclear for a reader, I'll try to explain. btf_encoder__find_function() guarantees that a particular encoder gets an elf_function struct that it "owns": meaning no other encoder touches it. Because of that btf_encoder__save_func() does not require any synchronization code. We know for sure that an encoder passed to it is always the owner of the elf_function passed to it, and so the elf_function can be safely modified.
