On Thu, Apr 24, 2025 at 5:48 PM Serhei Makarov <ser...@serhei.io> wrote:
>
> Changes for v4:
>
> - Separate out libdwfl_stacktrace, as requested.
>
> * * *
>
> 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.
>
> * libdwfl_stacktrace/libdwfl_stacktrace.h
> (Dwflst_Process_Tracker): New struct.
> (dwflst_tracker_begin): New function.
> (dwflst_tracker_dwfl_begin): New function.
> (dwflst_tracker_end): New function.
> * libdw/libdw.map: Add new functions.
> * libdwfl_stacktrace/libdwfl_stacktraceP.h
> (struct Dwflst_Process_Tracker): New struct.
> * libdwfl/libdwflP.h (Dwflst_Process_Tracker): Typedef forward decl.
> (struct Dwfl): Add 'tracker' field.
> * libdwfl_stacktrace/Makefile.am (libdwfl_stacktrace_a_SOURCES):
> Add dwflst_process_tracker.c.
> * libdwfl_stacktrace/dwflst_process_tracker.c: New file.
> (dwflst_tracker_begin): Initialize the tracker.
> (dwflst_tracker_dwfl_begin): Initialize Dwfl * with specified tracker.
> (dwflst_tracker_end): Deallocate the tracker.
> ---
> libdw/libdw.map | 4 ++
> libdwfl/libdwflP.h | 4 ++
> libdwfl_stacktrace/Makefile.am | 3 +-
> libdwfl_stacktrace/dwflst_process_tracker.c | 66 +++++++++++++++++++++
> libdwfl_stacktrace/libdwfl_stacktrace.h | 20 +++++++
> libdwfl_stacktrace/libdwfl_stacktraceP.h | 6 ++
> 6 files changed, 102 insertions(+), 1 deletion(-)
> create mode 100644 libdwfl_stacktrace/dwflst_process_tracker.c
>
> diff --git a/libdw/libdw.map b/libdw/libdw.map
> index 4f3fe6ba..fb69a62a 100644
> --- a/libdw/libdw.map
> +++ b/libdw/libdw.map
> @@ -396,4 +396,8 @@ ELFUTILS_0.193 {
> ELFUTILS_0.193_EXPERIMENTAL {
> global:
> dwflst_perf_sample_preferred_regs_mask;
> + dwflst_perf_sample_preferred_regs_mask;
> + dwflst_tracker_begin;
> + dwflst_tracker_dwfl_begin;
> + dwflst_tracker_end;
> };
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index 2b7eb8da..57305f81 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -111,9 +111,13 @@ struct Dwfl_User_Core
> int fd; /* close if >= 0. */
> };
>
> +/* forward decl from ../libdwfl_stacktrace/ */
> +typedef struct Dwflst_Process_Tracker Dwflst_Process_Tracker;
> +
> struct Dwfl
> {
> const Dwfl_Callbacks *callbacks;
> + Dwflst_Process_Tracker *tracker;
> #ifdef ENABLE_LIBDEBUGINFOD
> debuginfod_client *debuginfod;
> #endif
> diff --git a/libdwfl_stacktrace/Makefile.am b/libdwfl_stacktrace/Makefile.am
> index 6364c292..d57431c0 100644
> --- a/libdwfl_stacktrace/Makefile.am
> +++ b/libdwfl_stacktrace/Makefile.am
> @@ -40,7 +40,8 @@ noinst_LIBRARIES += libdwfl_stacktrace_pic.a
> pkginclude_HEADERS = libdwfl_stacktrace.h
>
>
> -libdwfl_stacktrace_a_SOURCES = dwflst_perf_frame.c
> +libdwfl_stacktrace_a_SOURCES = dwflst_process_tracker.c \
> + dwflst_perf_frame.c
>
> libdwfl_stacktrace = $(libdw)
> libdw = ../libdw/libdw.so
> diff --git a/libdwfl_stacktrace/dwflst_process_tracker.c
> b/libdwfl_stacktrace/dwflst_process_tracker.c
> new file mode 100644
> index 00000000..057c9f7a
> --- /dev/null
> +++ b/libdwfl_stacktrace/dwflst_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 "libdwfl_stacktraceP.h"
> +
> +Dwflst_Process_Tracker *dwflst_tracker_begin (const Dwfl_Callbacks
> *callbacks)
> +{
> + Dwflst_Process_Tracker *tracker = calloc (1, sizeof *tracker);
> + if (tracker == NULL)
> + {
> + __libdwfl_seterrno (DWFL_E_NOMEM);
> + return tracker;
> + }
> +
> + tracker->callbacks = callbacks;
> + return tracker;
> +}
> +
> +Dwfl *dwflst_tracker_dwfl_begin (Dwflst_Process_Tracker *tracker)
> +{
> + Dwfl *dwfl = INTUSE(dwfl_begin) (tracker->callbacks);
> + if (dwfl == NULL)
> + return dwfl;
> +
> + /* TODO: Could also share dwfl->debuginfod, but thread-safely? */
> + dwfl->tracker = tracker;
> + return dwfl;
> +}
> +
> +void dwflst_tracker_end (Dwflst_Process_Tracker *tracker)
> +{
> + if (tracker == NULL)
> + return;
> +
> + /* TODO: Call dwfl_end for each Dwfl connected to this tracker. */
> + free (tracker);
> +}
> diff --git a/libdwfl_stacktrace/libdwfl_stacktrace.h
> b/libdwfl_stacktrace/libdwfl_stacktrace.h
> index 564f504a..f3a82d18 100644
> --- a/libdwfl_stacktrace/libdwfl_stacktrace.h
> +++ b/libdwfl_stacktrace/libdwfl_stacktrace.h
> @@ -41,6 +41,26 @@
> extern "C" {
> #endif
>
> +/* Keeps track of and caches Elf structs across multiple libdwfl
> + sessions corresponding to different processes. */
> +typedef struct Dwflst_Process_Tracker Dwflst_Process_Tracker;
> +
> +
> +/* Initialize a new tracker for multiple libdwfl sessions. Since Elf
> + data will shared between the libdwfl sessions, each Dwfl must use
> + the same Dwfl_Callbacks CALLBACKS provided when the tracker is
> + created. */
> +extern Dwflst_Process_Tracker *dwflst_tracker_begin (const Dwfl_Callbacks
> *callbacks)
> + __nonnull_attribute__ (1);
> +
> +/* Create a new Dwfl linked to this tracker. */
> +extern Dwfl *dwflst_tracker_dwfl_begin (Dwflst_Process_Tracker *tracker)
> + __nonnull_attribute__ (1);
> +
> +/* End all sessions with this tracker. */
> +extern void dwflst_tracker_end (Dwflst_Process_Tracker *tracker);
> +
> +
> /* XXX dwflst_perf_sample_getframes to be added in subsequent patch */
>
> /* Returns the linux perf_events register mask describing a set of
> diff --git a/libdwfl_stacktrace/libdwfl_stacktraceP.h
> b/libdwfl_stacktrace/libdwfl_stacktraceP.h
> index fe6945fc..9313176c 100644
> --- a/libdwfl_stacktrace/libdwfl_stacktraceP.h
> +++ b/libdwfl_stacktrace/libdwfl_stacktraceP.h
> @@ -33,4 +33,10 @@
>
> #include "libdwflP.h"
>
> +struct Dwflst_Process_Tracker
> +{
> + const Dwfl_Callbacks *callbacks;
> + /* ... */
> +};
> +
> #endif /* libdwfl_stacktraceP.h */
> --
> 2.47.0
>
These parts of the Dwflst_Process_Tracker implementation LGTM.
Aaron
