[PATCH v3 5/8] Add makedumpfile extension support
Stephen Brennan
stephen.s.brennan at oracle.com
Thu Mar 12 15:24:19 PDT 2026
Tao Liu <ltao at redhat.com> writes:
> 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.
The key to making GCC generate the symbols for the sections is that the
section names should themselves be valid C identifiers (i.e. they should
not start with a '.'). Here's a patch to give the idea:
>From 3618f51869e978328e49f6061eae94469bca43c5 Mon Sep 17 00:00:00 2001
From: Stephen Brennan <stephen.s.brennan at oracle.com>
Date: Wed, 11 Mar 2026 09:17:05 -0700
Subject: [PATCH] Use automatically generated section start symbols
GCC will create __start_SECTION and __stop_SECTION only if the section
names are valid C identifiers. This means we cannot use a leading dot.
Drop the leading dot from the section names so we can avoid defining a
custom linker script.
Signed-off-by: Stephen Brennan <stephen.s.brennan at oracle.com>
---
Makefile | 2 +-
btf_info.h | 4 ++--
extensions/Makefile | 2 +-
kallsyms.h | 2 +-
makedumpfile.ld | 15 ---------------
5 files changed, 5 insertions(+), 20 deletions(-)
delete mode 100644 makedumpfile.ld
diff --git a/Makefile b/Makefile
index 8196aa6..e2cde8f 100644
--- a/Makefile
+++ b/Makefile
@@ -113,7 +113,7 @@ $(OBJ_ARCH): $(SRC_ARCH)
$(CC) $(CFLAGS_ARCH) -c -o ./$@ $(VPATH)$(@:.o=.c)
makedumpfile: $(SRC_BASE) $(OBJ_PART) $(OBJ_ARCH)
- $(CC) $(CFLAGS) $(LDFLAGS) $(OBJ_PART) $(OBJ_ARCH) -rdynamic -Wl,-T,makedumpfile.ld -o $@ $< $(LIBS)
+ $(CC) $(CFLAGS) $(LDFLAGS) $(OBJ_PART) $(OBJ_ARCH) -rdynamic -o $@ $< $(LIBS)
@sed -e "s/@DATE@/$(DATE)/" \
-e "s/@VERSION@/$(VERSION)/" \
$(VPATH)makedumpfile.8.in > $(VPATH)makedumpfile.8
diff --git a/btf_info.h b/btf_info.h
index 2a78fbf..555f21e 100644
--- a/btf_info.h
+++ b/btf_info.h
@@ -29,7 +29,7 @@ void cleanup_btf(void);
struct ktype_info _##MOD##_##S##_##M = { \
QUATE(MOD), QUATE(S), QUATE(M), 0, 0, 0, 0, R, R\
}; \
- __attribute__((section(".init_ktypes"), used)) \
+ __attribute__((section("init_ktypes"), used)) \
struct ktype_info * _ptr_##MOD##_##S##_##M = &_##MOD##_##S##_##M
#define INIT_MOD_STRUCT_MEMBER(MOD, S, M) \
@@ -60,7 +60,7 @@ void cleanup_btf(void);
struct ktype_info _##MOD##_##S = { \
QUATE(MOD), QUATE(S), 0, 0, 0, 0, 0, R, 0 \
}; \
- __attribute__((section(".init_ktypes"), used)) \
+ __attribute__((section("init_ktypes"), used)) \
struct ktype_info * _ptr_##MOD##_##S = &_##MOD##_##S
#define INIT_MOD_STRUCT(MOD, S) INIT_MOD_STRUCT_RQD(MOD, S, 1)
diff --git a/extensions/Makefile b/extensions/Makefile
index fc28577..a4dc86a 100644
--- a/extensions/Makefile
+++ b/extensions/Makefile
@@ -8,7 +8,7 @@ amdgpu_filter.so: maple_tree.c
userstack.so: maple_tree.c vma_rbtree.c
$(CONTRIB_SO): %.so: %.c
- $(CC) -O2 -g -fPIC -shared -Wl,-T,../makedumpfile.ld -o $@ $^
+ $(CC) -O2 -g -fPIC -shared -o $@ $^
clean:
rm -f $(CONTRIB_SO)
diff --git a/kallsyms.h b/kallsyms.h
index d94830e..1989dfe 100644
--- a/kallsyms.h
+++ b/kallsyms.h
@@ -17,7 +17,7 @@ struct ksym_info {
struct ksym_info _##MOD##_##SYM = { \
QUATE(MOD), QUATE(SYM), 0 \
}; \
- __attribute__((section(".init_ksyms"), used)) \
+ __attribute__((section("init_ksyms"), used)) \
struct ksym_info * _ptr_##MOD##_##SYM = &_##MOD##_##SYM
#define GET_MOD_SYM(MOD, SYM) (_##MOD##_##SYM.value)
diff --git a/makedumpfile.ld b/makedumpfile.ld
deleted file mode 100644
index 231a162..0000000
--- a/makedumpfile.ld
+++ /dev/null
@@ -1,15 +0,0 @@
-SECTIONS
-{
- .init_ksyms ALIGN(8) : {
- __start_init_ksyms = .;
- KEEP(*(.init_ksyms*))
- __stop_init_ksyms = .;
- }
-
- .init_ktypes ALIGN(8) : {
- __start_init_ktypes = .;
- KEEP(*(.init_ktypes*))
- __stop_init_ktypes = .;
- }
-}
-INSERT AFTER .data;
\ No newline at end of file
--
2.47.3
>> * 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.
Totally agreed up to here! I think it makes sense for plugin failures to
be independent.
It seems like the BTF loading process should not fail if any type or
member is not found. But once we've tried to load everything, we can
then go through each plugin's list, and if we find a "required" type
or member which is not present, we log that failure and don't run the
plugin. This shouldn't be much work, because we already have all the
types in an array for each plugin.
> 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.
Personally, I don't believe we need sandbox. The plugins should be
written to handle the clear error cases:
- Optional types or symbols may not be present, so validate them.
- Values read from /proc/vmcore may not be valid, or the read may fail,
so check all return values.
That said, I'm not set against it, so if you believe it's necessary I
could be convinced :)
Thanks again,
Stephen
>>
>> 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