[PATCH v3 5/8] Add makedumpfile extension support
Tao Liu
ltao at redhat.com
Wed Mar 11 07:41:22 PDT 2026
Hi Stephen,
On Wed, Mar 11, 2026 at 1:38 PM Stephen Brennan
<stephen.s.brennan at oracle.com> wrote:
>
> Tao Liu <ltao at redhat.com> writes:
> > Hi Stephen,
> >
> > Sorry I took some time to make modifications on v3 code. Please see a
> > drafted v4 [1] for a preview.
> >
> > I followed your suggestion to let extensions declare the kallsyms
> > symbol/btf types it needed, then during kallsyms/btf initialization,
> > it will only resolve the declared symbol/types, thus getting rid of
> > the hash table. Extensions now don't need to initial needed
> > symbols/types by itself.
> >
> > In addition, users can specify which extensions to load at
> > makedumpfile cmdline as:
> >
> > $ ./makedumpfile -d 31 -l
> > /var/crash/127.0.0.1-2025-06-10-18\:03\:12/vmcore /tmp/out --extension
> > amdgpu_filter.so
> >
> > Also "--extension" can be used several times, and amdgpu_filter.so can
> > be an absolute path as well. If no extensions are specified, the
> > btf/kallsyms of makedumpfile will not initialize.
> >
> > I don't know if this is what you wanted, or any suggestions for this?
> > Thanks in advance!
>
> Hi Tao,
>
> Please accept my apologies for the long delay on this review. I've gone through
> each commit with my feedback. This looks like a really excellent job and I
> think it is ready for the review of the maintainers. I've implemented my own
> userspace stack extension on top of this support, and I find it to be great for
> my use case as well, with one small change (adding flags for detecting whether a
> struct or member is found).
No worries at all :) Thanks a lot for your detailed comments. I'm
looking through them as well as your code branch in github. This may
take a while...
>
> Here is some more detailed feedback on a per-commit level; it's all pretty
> minor.
>
> d3aee7a ("Reserve sections for makedumpfile and extenions")
>
> * This is just a note, but I've found that the linker script may not be
> necessary. GCC automatically creates the __start_SECTION and __stop_SECTION
> symbols for sections it creates.
I have tried to remove the makedumpfile.ld linker script. The
automatic symbol as __start/stop_SECTION are only generated for
makedumpfile, aka the main program, but not for .so extensions. This
is a breaker because I would expect all .so files have the
__start/stop_SECTION symbol so they can be resigtered to main program.
I'm not expert in GCC's default behaviours, so I guess I would prefer
the current approach of explicit symbol define within the linker
script, seems to be more robust... Perhaps you can share your code on
this, I will give it a try.
> * Otherwise, this is exactly what I was thinking!
>
> 102ae0b ("Implement kernel kallsyms resolving")
>
> * Again, this is looking very close to the design I hoped for, thanks!
> * I'm not sure whether is_unwanted_symbol() needs to exist anymore? Given that
> users opt-in to specific symbols, we don't need to filter out noisy ones that
> would waste memory. What do you think?
Agreed, I can remove this function.
> * Unfortunately, upstream has made some changes to the vmlinux kallsyms encoding
> in 7.0. You may want to check what we did in drgn to support those changes:
> https://github.com/osandov/drgn/commit/744f36ec3c3f64d7e1323a0037898158698585c4
> * In kallsyms.c:
Thanks for the info, I will try this and update the code in v4.
>
> +/*
> + * Makedumpfile's .init_ksyms section
> +*/
> +extern struct ksym_info __start_init_ksyms[];
> +extern struct ksym_info __stop_init_ksyms[];
>
> I'm not sure it matters, but the type here is wrong. It should be
> "extern struct ksym_info *" because you're storing pointers, not the actual
> struct. That said, the type isn't used so I don't know that it matters.
Right, this is an error. Thanks for pointing that out.
>
> 4187b33 ("Implement kernel btf resolving")
>
> * The get_ktype_info() function fails if either the struct or member is not
> found. This makes sense in a lot of cases, but there are other cases where we
> will want to use the presence or absence of a struct/struct member to detect
> which version of code to use. For example, my userspace stack code will use
> either maple or rbtree helper code for finding stack VMAs, depending on which
> is available. We can't know until runtime, when we check whether mm_rb or
> mm_mt is present.
>
> One solution is to handle this at runtime. We can have a macro like
> "HAVE_MEMBER(S, M)" and "HAVE_STRUCT(S)", and then each extension can check
> whether for the members it expects to be present. I think the major downside
> to this approach is that it requires manual effort, which is likely to be
> forgotten when writing extensions.
>
> Alternatively, we could set a flag in the struct ktype_info if the type is
> "required" (or optional), and only fail get_ktype_info() for required
Agreed, an optional flag looks better.
> structs/members. The concern with this approach is: what if plugin (A)
> requires a type which is not present, but plugin (B) does not? If both are
> loaded, the failure of A would cause B not to run. I'm not sure whether we
> should care about that situation... I don't know if we have a use case for
> using multiple plugins at the same time. Until we do, we probably won't have a
> good idea whether it should be allowed for one to fail, but the other to
> continue.
Great question! I have thought about this previously. I suggest that
if plugin(A) fails, it should just fail and allow the execution of any
later plugins (B, C....) to continue. Each plugin is responsible for
one task, like plugin(A) for dealing with amdgpu's mm page
filtering,and plugin(B) for Intel's and plugin(C) for NV's. Plugin(A)
certainly will fail if one machine have no amdgpu, thus the amdgpu.ko
will never been loaded, so related symbol/types missing. This is
expected and shouldn't block the later plugins.
But the "fail" should gentle, not like the ones as segfault, which
will crash the entire makedumpfile program. Since currently plugins
are native .so libraries, the quality of code is ensured by each
plugin authors, rather than makedumpfile maintainers. Idealy the the
plugins are well tested in 1st kernel before they are shipped to kdump
img, but who knows. From makedumpfile's view, do you think we need to
introduce a sandbox to isolate plugins from makedumpfile? This would
prevent serious plugin errors from stopping makedumpfile from
generating the vmcore.
>
> I've implemented this second alternative in my branch.
>
> * Similar to the previous commit, __start_init_ktypes and __stop_init_ktypes are
> declared as structs but should probably be declared as pointers.
>
> 22097b7 ("Implement kernel module's kallsyms resolving")
> edfa698 ("Implement kernel module's btf resolving")
>
> * These commits are straightforward:
> Reviewed-by: Stephen Brennan <stephen.s.brennan at oracle.com>
>
> ede22e8 ("Add makedumpfile extensions support")
I will address those later after reading through your code branch.
Thanks again for your detailed comments!
Thanks,
Tao Liu
>
> * nit: the array "handlers" actually contains "handles" (no r) returned from
> dlopen(). The word handlers usually implies a function pointer or something
> like that, called to handle a certain situation. Maybe rename this to
> "handles".
> * The run_extensions() function is called within a retry loop in
> create_dumpfile(). It seems possible that this could get called multiple
> times. But many of the global variables in the kallsyms, BTF, and extension
> code are not safe to be cleaned up and reinitialized. In particular, when
> array elements are freed, the lengths, capacities, and pointers are not reset
> to 0/NULL. I think it would be wise to make all the cleanup functions clear
> the globals, so that they may be reinitialized safely.
> * Further, I think it might be helpful to split extension loading, running, and
> cleanup. Something like this in create_dumpfile():
>
> load_extensions(); /* loads everything and calls entry() */
> retry:
> /* ... create bitmap and dump ... */
> if (status == NOSPACE) {
> /* ... */
> goto retry;
> }
> /* ... */
> cleanup_extensions();
> return;
>
> For two reasons: (1) this avoids unnecessary work re-loading and
> re-initializing the BTF, kallsyms, and extensions. (Though I still think it's
> safer to ensure they can be re-initialized safely.) And (2), this allows for
> future use of extensions in the rest of the dump operation. For my userspace
> stack extension, I plan to add a callback which allows extensions to override
> the decision to filter a page, since my logic can't be easily done via erase
> info. So the extensions need to remain loaded during the creation of the
> dumpfile, and cleaned up after. I have tweaked this in my own patches,
> but I just wanted to share the use case.
>
> c568635 ("btf/kallsyms based makedumpfile extension for mm page filtering")
>
> * This looks good to me!
> * I will say that for my userspace stack use case, while the "filter_page(...,
> false)" mechanism for specifying pages that are retained *looks* useful, I
> wouldn't be able to use it because I cannot determine the PFNs for each stack
> VMA. Instead, I have to use the page "mapping" and "index" fields to match the
> VMA and determine whether the PFN falls in a range I care to save. Of course,
> just because *I* won't use it doesn't mean it's not useful :)
>
> 2b252ec ("Filter amdgpu mm pages")
>
> * I'm no expert in amdgpu, but the overall approach makes sense to me, and the
> helpers look good.
>
>
> At a broader level, while the add_to_arr() function is useful, I do think the
> dynamic array/vector pattern could be captured with a dedicated data structure.
> For instance, drgn has this excellent LGPL-2.1+ header-only vector library:
> https://github.com/osandov/drgn/blob/main/libdrgn/vector.h
> I don't think it is high priority or must be addressed. Nor do I mean that this
> particular implementation choice be used. It's just something to share & think
> about.
>
> To provide some context on my comments related to the userspace stack tracing,
> here is a branch of mine which is based on yours, that adds my userspace stack
> extension and a few tweaks:
>
> https://github.com/brenns10/makedumpfile/commits/stepbren_userstack_v4/
>
> Thank you,
> Stephen
>
> > Thanks,
> > Tao Liu
> >
> >
> >
> >
> >
> >
> >
> > [1]: https://github.com/liutgnu/makedumpfile/commits/v4/
> >
> > On Fri, Jan 23, 2026 at 2:43 AM Tao Liu <ltao at redhat.com> wrote:
> >>
> >> Hi Stephen,
> >>
> >> Thanks a lot for your quick reply and detailed information, I really
> >> appreciate it!
> >>
> >> On Thu, Jan 22, 2026 at 1:51 PM Stephen Brennan
> >> <stephen.s.brennan at oracle.com> wrote:
> >> >
> >> > Hi Tao,
> >> >
> >> > This series looks really great -- I'm excited to see the switch to
> >> > native .so extensions instead of epicc. I've applied the series locally
> >> > and I'll rebuild my userspace stack inclusion feature based on it, to
> >> > try it out myself.
> >>
> >> Awesome, looking forward to your feedback on the code/API designs etc...
> >>
> >> >
> >> > In the meantime, I'll share some of my feedback on the patches (though
> >> > I'm not a makedumpfile developer). This seems like the most important
> >> > patch in terms of design, so I'll start here.
> >> >
> >> > Tao Liu <ltao at redhat.com> writes:
> >> > > This patch will add .so extension support to makedumpfile, similar to crash
> >> > > extension to crash utility. Currently only "/usr/lib64/makedumpfile/extensions"
> >> > > and "./extensions" are searched for extensions. Once found, kallsyms and btf
> >> > > will be initialized so all extensions can benifit from it (Currently makedumpfile
> >> > > doesn't use these info, we can move the kallsyms/btf init code else where later
> >> > > if makedumpfile needs them).
> >> > >
> >> > > The makedumpfile extension is to help users to customize mm page filtering upon
> >> > > traditional mm page flag filtering, without make code modification on makedumpfile
> >> > > itself.
> >> > >
> >> > > Signed-off-by: Tao Liu <ltao at redhat.com>
> >> > > ---
> >> > > Makefile | 7 +++-
> >> > > extension.c | 82 +++++++++++++++++++++++++++++++++++++++++++++
> >> > > extensions/Makefile | 10 ++++++
> >> > > makedumpfile.c | 4 +++
> >> > > 4 files changed, 102 insertions(+), 1 deletion(-)
> >> > > create mode 100644 extension.c
> >> > > create mode 100644 extensions/Makefile
> >> > >
> >> > > diff --git a/Makefile b/Makefile
> >> > > index f3f4da8..7e29220 100644
> >> > > --- a/Makefile
> >> > > +++ b/Makefile
> >> > > @@ -45,7 +45,7 @@ CFLAGS_ARCH += -m32
> >> > > endif
> >> > >
> >> > > SRC_BASE = makedumpfile.c makedumpfile.h diskdump_mod.h sadump_mod.h sadump_info.h
> >> > > -SRC_PART = print_info.c dwarf_info.c elf_info.c erase_info.c sadump_info.c cache.c tools.c printk.c detect_cycle.c kallsyms.c btf_info.c
> >> > > +SRC_PART = print_info.c dwarf_info.c elf_info.c erase_info.c sadump_info.c cache.c tools.c printk.c detect_cycle.c kallsyms.c btf_info.c extension.c
> >> > > OBJ_PART=$(patsubst %.c,%.o,$(SRC_PART))
> >> > > SRC_ARCH = arch/arm.c arch/arm64.c arch/x86.c arch/x86_64.c arch/ia64.c arch/ppc64.c arch/s390x.c arch/ppc.c arch/sparc64.c arch/mips64.c arch/loongarch64.c arch/riscv64.c
> >> > > OBJ_ARCH=$(patsubst %.c,%.o,$(SRC_ARCH))
> >> > > @@ -126,6 +126,7 @@ eppic_makedumpfile.so: extension_eppic.c
> >> > >
> >> > > clean:
> >> > > rm -f $(OBJ) $(OBJ_PART) $(OBJ_ARCH) makedumpfile makedumpfile.8 makedumpfile.conf.5
> >> > > + $(MAKE) -C extensions clean
> >> > >
> >> > > install:
> >> > > install -m 755 -d ${DESTDIR}/${SBINDIR} ${DESTDIR}/usr/share/man/man5 ${DESTDIR}/usr/share/man/man8
> >> > > @@ -135,3 +136,7 @@ install:
> >> > > mkdir -p ${DESTDIR}/usr/share/makedumpfile/eppic_scripts
> >> > > install -m 644 -D $(VPATH)makedumpfile.conf ${DESTDIR}/usr/share/makedumpfile/makedumpfile.conf.sample
> >> > > install -m 644 -t ${DESTDIR}/usr/share/makedumpfile/eppic_scripts/ $(VPATH)eppic_scripts/*
> >> > > +
> >> > > +.PHONY: extensions
> >> > > +extensions:
> >> > > + $(MAKE) -C extensions CC=$(CC)
> >> > > \ No newline at end of file
> >> > > diff --git a/extension.c b/extension.c
> >> > > new file mode 100644
> >> > > index 0000000..6ee7f4e
> >> > > --- /dev/null
> >> > > +++ b/extension.c
> >> > > @@ -0,0 +1,82 @@
> >> > > +#include <stdio.h>
> >> > > +#include <stdlib.h>
> >> > > +#include <string.h>
> >> > > +#include <dirent.h>
> >> > > +#include <dlfcn.h>
> >> > > +#include <stdbool.h>
> >> > > +#include "kallsyms.h"
> >> > > +#include "btf_info.h"
> >> > > +
> >> > > +static const char *dirs[] = {
> >> > > + "/usr/lib64/makedumpfile/extensions",
> >> > > + "./extensions",
> >> > > +};
> >> > > +
> >> > > +/* Will only init once */
> >> > > +static bool init_kallsyms_btf(void)
> >> > > +{
> >> > > + static bool ret = false;
> >> > > + static bool has_inited = false;
> >> > > +
> >> > > + if (has_inited)
> >> > > + goto out;
> >> > > + if (!init_kernel_kallsyms())
> >> > > + goto out;
> >> > > + if (!init_kernel_btf())
> >> > > + goto out;
> >> > > + if (!init_module_kallsyms())
> >> > > + goto out;
> >> > > + if (!init_module_btf())
> >> > > + goto out;
> >> > > + ret = true;
> >> >
> >> > I feel it would be good practice to load as little information as is
> >> > necessary for the task. If "amdgpu" module is required, then load kernel
> >> > kallsyms, BTF, and then the amdgpu module kallsyms & BTF. If no module
> >> > debuginfo is required, then just the kernel would suffice.
> >> >
> >> > This would reduce memory usage and runtime, though I don't know if it
> >> > would show up in profiling. The main benefit could be reliability: by
> >> > handling less data, there are fewer chances to hit an error.
> >>
> >> OK, I agree, mandatory kernel btf/kallsyms info + optional kernel
> >> module btf/kallsyms info is a reasonable design. So kernel modules'
> >> info can be loaded on demand.
> >>
> >> >
> >> > > +out:
> >> > > + has_inited = true;
> >> > > + return ret;
> >> > > +}
> >> > > +
> >> > > +static void cleanup_kallsyms_btf(void)
> >> > > +{
> >> > > + cleanup_kallsyms();
> >> > > + cleanup_btf();
> >> > > +}
> >> > > +
> >> > > +void run_extensions(void)
> >> > > +{
> >> > > + DIR *dir;
> >> > > + struct dirent *entry;
> >> > > + size_t len;
> >> > > + int i;
> >> > > + void *handle;
> >> > > + char path[512];
> >> > > +
> >> > > + for (i = 0; i < sizeof(dirs) / sizeof(char *); i++) {
> >> > > + if ((dir = opendir(dirs[i])) != NULL)
> >> > > + break;
> >> > > + }
> >> > > +
> >> > > + if (!dir || i >= sizeof(dirs) / sizeof(char *))
> >> > > + /* No extensions found */
> >> > > + return;
> >> >
> >> > It could be confusing that makedumpfile would behave differently with
> >> > the same command-line arguments depending on the presence or absence of
> >> > these extensions on the filesystem.
> >> >
> >> > I think it may fit users' expectations better if they are required to
> >> > specify extensions on the command line. Then we could load them by
> >> > searching each directory in order. This allows:
> >> >
> >> > (a) more expected behavior
> >> > (b) multiple extensions can exist without all being enabled, thus more
> >> > flexibility
> >> > (c) extensions can be present in the local "extensions/" directory, or
> >> > in the system directory
> >>
> >> Sure, it also sounds reasonable. My original thoughts are, user
> >> customization on mm filtering are specified in .so, and if user don't
> >> need one .so, e.g. amdgpu mm filtering for a nvidia machine, then he
> >> doesn't pack the amdgpu_filter.so into kdump's initramfs. I agree
> >> adding extra makedumpfile cmdline option to receive those needed .so
> >> is a better design.
> >>
> >> >
> >> > > + while ((entry = readdir(dir)) != NULL) {
> >> > > + len = strlen(entry->d_name);
> >> > > + if (len > 3 && strcmp(entry->d_name + len - 3, ".so") == 0) {
> >> > > + /* Will only init when .so exist */
> >> > > + if (!init_kallsyms_btf())
> >> > > + goto out;
> >> > > +
> >> > > + snprintf(path, sizeof(path), "%s/%s", dirs[i], entry->d_name);
> >> > > + handle = dlopen(path, RTLD_NOW);
> >> > > + if (!handle) {
> >> > > + fprintf(stderr, "%s: Failed to load %s: %s\n",
> >> > > + __func__, path, dlerror());
> >> > > + continue;
> >> > > + }
> >> > > + printf("Loaded extension: %s\n", path);
> >> > > + dlclose(handle);
> >> >
> >> > Using the constructor/destructor of the shared object is clever! But we
> >> > lose some flexibility: by the time the dlopen() returns, the constructor
> >> > has executed and the plugin has thus executed.
> >> >
> >> > What if we instead use dlsym() to load some symbols from the DSO? In
> >> > particular, I think it would be useful if extensions could declare a
> >> > list of symbols and a list of structure information which they are
> >> > interested in receiving. We could use these lists to know which
> >> > kernel/module kallsyms & BTF we should load. We could even load the
> >> > information into the local variables of the extension, so the extension
> >> > would not need to manually load it.
> >> >
> >> > Of course this is more complex, but the benefit is:
> >> >
> >> > 1. Extensions can be written more simply, and would not need to manually
> >> > load each symbol & type.
> >> > 2. We could eliminate the hash tables for kallsyms & BTF, and eliminate
> >> > the loading of unnecessary module information. Instead, we'd just
> >> > populate the symbol addresses, struct offsets, and type sizes directly
> >> > into the local variables which request them.
> >>
> >> It is a clever idea! Though complex for code, I think it is doable.
> >>
> >> >
> >> > Again, while I don't want to prematurely optimize -- it's good to avoid
> >> > loading unnecessary information. I hope I've described my idea well. I
> >> > would be happy to work on an implementation of it based on your patches
> >> > here, if you're interested.
> >>
> >> Thanks again for your suggestions! I got your points and I think I can
> >> improve the code while waiting for maintainers ideas at the same time.
> >> I will let you know when done or encounter blockers if any.
> >>
> >> Thanks,
> >> Tao Liu
> >>
> >> >
> >> > Thanks,
> >> > Stephen
> >> >
> >> > > + }
> >> > > + }
> >> > > +out:
> >> > > + closedir(dir);
> >> > > + cleanup_kallsyms_btf();
> >> > > +}
> >> > > \ No newline at end of file
> >> > > diff --git a/extensions/Makefile b/extensions/Makefile
> >> > > new file mode 100644
> >> > > index 0000000..afbc61e
> >> > > --- /dev/null
> >> > > +++ b/extensions/Makefile
> >> > > @@ -0,0 +1,10 @@
> >> > > +CC ?= gcc
> >> > > +CONTRIB_SO :=
> >> > > +
> >> > > +all: $(CONTRIB_SO)
> >> > > +
> >> > > +$(CONTRIB_SO): %.so: %.c
> >> > > + $(CC) -O2 -g -fPIC -shared -o $@ $^
> >> > > +
> >> > > +clean:
> >> > > + rm -f $(CONTRIB_SO)
> >> > > diff --git a/makedumpfile.c b/makedumpfile.c
> >> > > index dba3628..ca8ed8a 100644
> >> > > --- a/makedumpfile.c
> >> > > +++ b/makedumpfile.c
> >> > > @@ -10847,6 +10847,8 @@ update_dump_level(void)
> >> > > }
> >> > > }
> >> > >
> >> > > +void run_extensions(void);
> >> > > +
> >> > > int
> >> > > create_dumpfile(void)
> >> > > {
> >> > > @@ -10884,6 +10886,8 @@ retry:
> >> > > if (info->flag_refiltering)
> >> > > update_dump_level();
> >> > >
> >> > > + run_extensions();
> >> > > +
> >> > > if ((info->name_filterconfig || info->name_eppic_config)
> >> > > && !gather_filter_info())
> >> > > return FALSE;
> >> > > --
> >> > > 2.47.0
> >> >
>
More information about the kexec
mailing list