[PATCH v3 5/8] Add makedumpfile extension support
Stephen Brennan
stephen.s.brennan at oracle.com
Tue Mar 10 17:38:27 PDT 2026
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).
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.
* 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?
* 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:
+/*
+ * 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.
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
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.
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")
* 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