Hi Serhei,
On Sun, 2025-03-16 at 19:14 -0400, Serhei Makarov wrote:
> New data structure to coordinate caching Elf data among multiple Dwfl
> structs attached to different processes. Meant to reduce the overhead
> for profilers that use elfutils for unwinding.
>
> The caching is well-justified, as the prior approach (e.g. in
> eu-stacktrace, sysprof-live-unwinder) of creating a separate Dwfl per
> process was wildly redundant, opening ~hundreds of file descriptors
> just for each common library such as libc.so and leaking the data.
>
> Initial patch just introduces the struct, to be filled in by the rest
> of the patch series.
I am wondering whether this isn't too abstract. libdwfl Dwfl's are
already super abstract so they can be used for running processes and
core dump and an offline or running kernel. Which is super powerful,
but also slightly confusing at times.
Can a Dwfl_Process_Tracker really take any callbacks and any Dwfl? Or
would it make sense to limit it (as its name already implies) to Dwfls
that report through dwfl_linux_proc_report with a given pid?
If so we might be able to simplify the interface a little and make it
slightly more intuitive to use.
Cheers,
Mark
> * libdwfl/libdwfl.h (Dwfl_Process_Tracker): New struct.
> (dwfl_process_tracker_begin): New function.
> (dwfl_begin_with_tracker): New function.
> (dwfl_process_tracker_end): New function.
> * libdw/libdw.map: Add new functions.
> * libdwfl/libdwflP.h (struct Dwfl_Process_Tracker): New struct.
> (struct Dwfl): Add 'tracker' field.
> * libdwfl/Makefile.am (libdwfl_a_SOURCES): Add dwfl_process_tracker.c.
> * libdwfl/dwfl_process_tracker.c: New file.
> (dwfl_process_tracker_begin): Initialize the tracker.
> (dwfl_begin_with_tracker): Initialize Dwfl * with specified tracker.
> (dwfl_process_tracker_end): Deallocate the tracker.
> ---
> libdw/libdw.map | 3 ++
> libdwfl/Makefile.am | 3 +-
> libdwfl/dwfl_process_tracker.c | 66 ++++++++++++++++++++++++++++++++++
> libdwfl/libdwfl.h | 17 ++++++++-
> libdwfl/libdwflP.h | 9 ++++-
> 5 files changed, 95 insertions(+), 3 deletions(-)
> create mode 100644 libdwfl/dwfl_process_tracker.c
>
> diff --git a/libdw/libdw.map b/libdw/libdw.map
> index 3218fd9c..ad3ec590 100644
> --- a/libdw/libdw.map
> +++ b/libdw/libdw.map
> @@ -390,4 +390,7 @@ ELFUTILS_0.193 {
> global:
> dwfl_set_initial_registers_thread;
> dwfl_perf_sample_preferred_regs_mask;
> + dwfl_process_tracker_begin;
> + dwfl_begin_with_tracker;
> + dwfl_process_tracker_end;
> } ELFUTILS_0.192;
> diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
> index 37c57bee..b41122e3 100644
> --- a/libdwfl/Makefile.am
> +++ b/libdwfl/Makefile.am
> @@ -71,7 +71,8 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c
> dwfl_version.c \
> link_map.c core-file.c open.c image-header.c \
> dwfl_frame.c frame_unwind.c dwfl_frame_pc.c \
> linux-pid-attach.c linux-core-attach.c dwfl_frame_regs.c \
> - dwfl_perf_frame.c \
> + dwfl_process_tracker.c \
> + dwfl_perf_frame.c \
> gzip.c debuginfod-client.c
>
> if BZLIB
> diff --git a/libdwfl/dwfl_process_tracker.c b/libdwfl/dwfl_process_tracker.c
> new file mode 100644
> index 00000000..c42d8ad2
> --- /dev/null
> +++ b/libdwfl/dwfl_process_tracker.c
> @@ -0,0 +1,66 @@
> +/* Track multiple Dwfl structs for multiple processes.
> + Copyright (C) 2025, Red Hat, Inc.
> + This file is part of elfutils.
> +
> + This file is free software; you can redistribute it and/or modify
> + it under the terms of either
> +
> + * the GNU Lesser General Public License as published by the Free
> + Software Foundation; either version 3 of the License, or (at
> + your option) any later version
> +
> + or
> +
> + * the GNU General Public License as published by the Free
> + Software Foundation; either version 2 of the License, or (at
> + your option) any later version
> +
> + or both in parallel, as here.
> +
> + elfutils is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + General Public License for more details.
> +
> + You should have received copies of the GNU General Public License and
> + the GNU Lesser General Public License along with this program. If
> + not, see <http://www.gnu.org/licenses/>. */
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#include "libdwflP.h"
> +
> +Dwfl_Process_Tracker *dwfl_process_tracker_begin (const Dwfl_Callbacks
> *callbacks)
> +{
> + Dwfl_Process_Tracker *tracker = calloc (1, sizeof *tracker);
> + if (tracker == NULL)
> + {
> + __libdwfl_seterrno (DWFL_E_NOMEM);
> + return tracker;
> + }
> +
> + tracker->callbacks = callbacks;
> + return tracker;
> +}
> +
> +Dwfl *dwfl_begin_with_tracker (Dwfl_Process_Tracker *tracker)
> +{
> + Dwfl *dwfl = dwfl_begin (tracker->callbacks);
> + if (dwfl == NULL)
> + return dwfl;
> +
> + /* TODO: Could also share dwfl->debuginfod, but thread-safely? */
> + dwfl->tracker = tracker;
> + return dwfl;
> +}
> +
> +void dwfl_process_tracker_end (Dwfl_Process_Tracker *tracker)
> +{
> + if (tracker == NULL)
> + return;
> +
> + /* TODO: Call dwfl_end for each Dwfl connected to this tracker. */
> + free (tracker);
> +}
> diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
> index 29505863..86005515 100644
> --- a/libdwfl/libdwfl.h
> +++ b/libdwfl/libdwfl.h
> @@ -32,9 +32,12 @@
> #include "libdw.h"
> #include <stdio.h>
>
> -/* Handle for a session using the library. */
> +/* Handle for a session using the library to attach to a single target
> process. */
> typedef struct Dwfl Dwfl;
>
> +/* Handle for a session using the library to attach to more than one
> process. */
> +typedef struct Dwfl_Process_Tracker Dwfl_Process_Tracker;
> +
> /* Handle for a module. */
> typedef struct Dwfl_Module Dwfl_Module;
>
> @@ -122,6 +125,18 @@ extern int dwfl_errno (void);
> extern const char *dwfl_errmsg (int err);
>
>
> +/* Start a new multi-process session with the library. */
> +extern Dwfl_Process_Tracker *dwfl_process_tracker_begin (const
> Dwfl_Callbacks *callbacks)
> + __nonnull_attribute__ (1);
> +
> +/* Create a new Dwfl within a multi-process session. */
> +extern Dwfl *dwfl_begin_with_tracker (Dwfl_Process_Tracker *tracker)
> + __nonnull_attribute__ (1);
> +
> +/* End a multi-process session. */
> +extern void dwfl_process_tracker_end (Dwfl_Process_Tracker *tracker);
> +
> +
> /* Start reporting the current set of segments and modules to the library.
> All existing segments are wiped. Existing modules are marked to be
> deleted, and will not be found via dwfl_addrmodule et al if they are not
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index 6ec5c966..c63f7c7d 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -1,5 +1,5 @@
> /* Internal definitions for libdwfl.
> - Copyright (C) 2005-2015, 2018, 2024 Red Hat, Inc.
> + Copyright (C) 2005-2015, 2018, 2024-2025 Red Hat, Inc.
> This file is part of elfutils.
>
> This file is free software; you can redistribute it and/or modify
> @@ -101,6 +101,12 @@ typedef enum { DWFL_ERRORS DWFL_E_NUM } Dwfl_Error;
> extern int __libdwfl_canon_error (Dwfl_Error) internal_function;
> extern void __libdwfl_seterrno (Dwfl_Error) internal_function;
>
> +struct Dwfl_Process_Tracker
> +{
> + const Dwfl_Callbacks *callbacks;
> + /* ... */
> +};
> +
> /* Resources we might keep for the user about the core file that the
> Dwfl might have been created from. Can currently only be set
> through std-argp. */
> @@ -114,6 +120,7 @@ struct Dwfl_User_Core
> struct Dwfl
> {
> const Dwfl_Callbacks *callbacks;
> + Dwfl_Process_Tracker *tracker;
> #ifdef ENABLE_LIBDEBUGINFOD
> debuginfod_client *debuginfod;
> #endif
