Hi Aaron, On Fri, 2026-10-30 at 01:16 -0400, Aaron Merey wrote: > Add load_lock to Dwfl for serializing module loading, including loading > of module's elf, dwarf, symbols as well as performing relocations. > > load_lock is used only for lazy loading of these components. Hot paths > are kept lock-free using atomics. New Dwfl_Module fields elf_loaded, > dwarf_loaded and symbols_loaded have been added. These are set to > true when lazy loading of elf, dwarf and symbols is completed. They are > also set to true when initialization has been attempted and failed. > > This commit serializes only the lazy loading entry points. > > libdwfl/ > > * dwfl_begin.c (dwfl_begin): Initialize load_lock. Also move > the debuginfod_lock init to avoid NULL dereference. > * dwfl_end.c (dwfl_end): Tear down load_lock. > * dwfl_module_build_id.c (found_build_id): Store build_id_len > atomically. > (dwfl_module_build_id): Attempt to read build_id_len > atomically. If it hasn't been loaded, acquire load_lock > first. > * dwfl_module_getdwarf.c (find_symtab_locked): Renamed from > find_symtab. > (find_symtab): Wrapper for find_symtab_locked that handles > locking and checking symbols_loaded. > (get_dwarf_locked): Renamed from dwfl_module_getdwarf. > (dwfl_module_getdwarf): Wrapper for get_dwarf_locked that > handles locking and checking dwarf_loaded. > (__libdwfl_getelf): Reset mod->build_id_len atomically. > * dwfl_module_getelf.c (get_elf_locked): Renamed from > dwfl_module_getelf. > (dwfl_module_getelf): Wrapper for get_elf_locked that handles > locking and checking elf_loaded. > * dwfl_module_getsym.c (__libdwfl_getsym): Check symbols_loaded > before calling dwfl_module_getsymtab. > * libdwflP.h (struct Dwfl): Add load_lock. > (struct Dwfl_Module): Add bools symbols_loaded, dwarf_loaded > and elf_loaded. > > Signed-off-by: Aaron Merey <[email protected]> > --- > v2 changes: > Update dwarf_loaded in dwfl_module_getdwarf instead of > get_dwarf_locked.
This looks nicer/more consistent. > Also reset mod->build_id_len atomically in > __libdwfl_getelf. Missed that one. But ... > diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c > index 430c3236..77af5189 100644 > --- a/libdwfl/dwfl_module_getdwarf.c > +++ b/libdwfl/dwfl_module_getdwarf.c > @@ -237,7 +237,7 @@ __libdwfl_getelf (Dwfl_Module *mod) > We'll fetch it from the file when asked. */ > free (mod->build_id_bits); > mod->build_id_bits = NULL; > - mod->build_id_len = 0; > + atomic_store_release (&mod->build_id_len, 0); > } > else if (fallback) > mod_verify_build_id (mod); I am a little confused about how we end up in this situation. So, does this switch mod->build_id_len from != 0 to 0? Then it seems to break other callers to do atomic_load and checking whether or not it is zero. The pattern is to atomically check if build_id_len is zero, if not, you can continue with the value (or see that it is -1 for not found), otherwise you take the load mutex (again check if it is still zero), set the build_id and atomically update the build_id_len (to -1 if not found), then drop the lock. What am I missing here? Thanks, Mark
