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! Thanks, Tao Liu [1]: https://github.com/liutgnu/makedumpfile/commits/v4/ On Fri, Jan 23, 2026 at 2:43 AM Tao Liu <[email protected]> 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 > <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > --- > > > 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 > >
