Add function to register irq info into kmemdump.
Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
include/linux/irqnr.h | 1 +
kernel/irq/irqdesc.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/linux/irqnr.h b/include/linux/irqnr.h
index e97206c721a0..136f0572ad78 100644
--- a/include/linux/irqnr.h
+++ b/include/linux/irqnr.h
@@ -9,6 +9,7 @@ unsigned int irq_get_nr_irqs(void) __pure;
unsigned int irq_set_nr_irqs(unsigned int nr);
extern struct irq_desc *irq_to_desc(unsigned int irq);
unsigned int irq_get_next_irq(unsigned int offset);
+void irq_kmemdump_register(void);
#define for_each_irq_desc(irq, desc) \
for (unsigned int __nr_irqs__ = irq_get_nr_irqs(); __nr_irqs__; \
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 287830739783..ae29165b1f1f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -12,6 +12,7 @@
#include <linux/export.h>
#include <linux/interrupt.h>
#include <linux/kernel_stat.h>
+#include <linux/kmemdump.h>
#include <linux/maple_tree.h>
#include <linux/irqdomain.h>
#include <linux/sysfs.h>
@@ -164,6 +165,12 @@ unsigned int irq_set_nr_irqs(unsigned int nr)
}
EXPORT_SYMBOL_GPL(irq_set_nr_irqs);
+void irq_kmemdump_register(void)
+{
+ kmemdump_register("irq", (void *)&nr_irqs, sizeof(nr_irqs));
+}
+EXPORT_SYMBOL_GPL(irq_kmemdump_register);
+
static DEFINE_MUTEX(sparse_irq_lock);
static struct maple_tree sparse_irqs = MTREE_INIT_EXT(sparse_irqs,
MT_FLAGS_ALLOC_RANGE |
--
2.43.0
$Subject: ... See
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes
On Tue, Apr 22 2025 at 14:31, Eugen Hristev wrote:
> Add function to register irq info into kmemdump.
What is irq info? Please explain explicitly which information is exposed
and why.
>
> +void irq_kmemdump_register(void)
> +{
> + kmemdump_register("irq", (void *)&nr_irqs, sizeof(nr_irqs));
> +}
> +EXPORT_SYMBOL_GPL(irq_kmemdump_register);
Are you going to slap a gazillion of those register a single variable
functions all over the place?
That's a really horrible idea.
The obvious way to deal with that is to annotate the variable:
static unsigned int nr_irqs = NR_IRQS;
KMEMDUMP_VAR(nr_irqs);
Let KMEMDUMP_VAR() store the size and the address of 'nr_irqs' in a
kmemdump specific section and then kmemdump can just walk that section
and dump stuff. No magic register functions and no extra storage
management for static/global variables.
No?
Thanks,
tglx
On 5/7/25 13:18, Thomas Gleixner wrote:
>
> $Subject: ... See
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes
>
> On Tue, Apr 22 2025 at 14:31, Eugen Hristev wrote:
>> Add function to register irq info into kmemdump.
>
> What is irq info? Please explain explicitly which information is exposed
> and why.
>
>>
>> +void irq_kmemdump_register(void)
>> +{
>> + kmemdump_register("irq", (void *)&nr_irqs, sizeof(nr_irqs));
>> +}
>> +EXPORT_SYMBOL_GPL(irq_kmemdump_register);
>
> Are you going to slap a gazillion of those register a single variable
> functions all over the place?
>
> That's a really horrible idea.
>
> The obvious way to deal with that is to annotate the variable:
>
> static unsigned int nr_irqs = NR_IRQS;
> KMEMDUMP_VAR(nr_irqs);
>
> Let KMEMDUMP_VAR() store the size and the address of 'nr_irqs' in a
> kmemdump specific section and then kmemdump can just walk that section
> and dump stuff. No magic register functions and no extra storage
> management for static/global variables.
>
> No?
Thank you very much for your review ! I will try it out.
Eugen
>
> Thanks,
>
> tglx
On 5/7/25 13:27, Eugen Hristev wrote:
>
>
> On 5/7/25 13:18, Thomas Gleixner wrote:
>>
>> $Subject: ... See
>>
>> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-submission-notes
>>
>> On Tue, Apr 22 2025 at 14:31, Eugen Hristev wrote:
>>> Add function to register irq info into kmemdump.
>>
>> What is irq info? Please explain explicitly which information is exposed
>> and why.
>>
>>>
>>> +void irq_kmemdump_register(void)
>>> +{
>>> + kmemdump_register("irq", (void *)&nr_irqs, sizeof(nr_irqs));
>>> +}
>>> +EXPORT_SYMBOL_GPL(irq_kmemdump_register);
>>
>> Are you going to slap a gazillion of those register a single variable
>> functions all over the place?
>>
>> That's a really horrible idea.
>>
>> The obvious way to deal with that is to annotate the variable:
>>
>> static unsigned int nr_irqs = NR_IRQS;
>> KMEMDUMP_VAR(nr_irqs);
>>
>> Let KMEMDUMP_VAR() store the size and the address of 'nr_irqs' in a
>> kmemdump specific section and then kmemdump can just walk that section
>> and dump stuff. No magic register functions and no extra storage
>> management for static/global variables.
>>
>> No?
>
> Thank you very much for your review ! I will try it out.
I have tried this way and it's much cleaner ! thanks for the suggestion.
The thing that I am trying to figure out now is how to do something
similar for a dynamically allocated memory, e.g.
void *p = kmalloc(...);
and then I can annotate `p` itself, it's address and size, but what I
would also want to so dump the whole memory region pointed out by p. and
that area address and size cannot be figured out at compile time hence I
can't instantiate a struct inside the dedicated section for it.
Any suggestion on how to make that better ? Or just keep the function
call to register the area into kmemdump ?
Thanks again,
Eugen
>
> Eugen
>>
>> Thanks,
>>
>> tglx
>
On Fri, Jun 13 2025 at 17:33, Eugen Hristev wrote:
> On 5/7/25 13:27, Eugen Hristev wrote:
>>> Let KMEMDUMP_VAR() store the size and the address of 'nr_irqs' in a
>>> kmemdump specific section and then kmemdump can just walk that section
>>> and dump stuff. No magic register functions and no extra storage
>>> management for static/global variables.
>>>
>>> No?
>>
>> Thank you very much for your review ! I will try it out.
>
> I have tried this way and it's much cleaner ! thanks for the
> suggestion.
Welcome.
> The thing that I am trying to figure out now is how to do something
> similar for a dynamically allocated memory, e.g.
> void *p = kmalloc(...);
> and then I can annotate `p` itself, it's address and size, but what I
> would also want to so dump the whole memory region pointed out by p. and
> that area address and size cannot be figured out at compile time hence I
> can't instantiate a struct inside the dedicated section for it.
> Any suggestion on how to make that better ? Or just keep the function
> call to register the area into kmemdump ?
Right. For dynamically allocated memory there is obviously no compile
time magic possible.
But I think you can simplify the registration for dynamically allocated
memory significantly.
struct kmemdump_entry {
void *ptr;
size_t size;
enum kmemdump_uids uid;
};
You use that layout for the compile time table and the runtime
registrations.
I intentionally used an UID as that avoids string allocation and all of
the related nonsense. Mapping UID to a string is a post processing
problem and really does not need to be done in the kernel. The 8
character strings are horribly limited and a simple 4 byte unique id is
achieving the same and saving space.
Just stick the IDs into include/linux/kmemdump_ids.h and expose the
content for the post processing machinery.
So you want KMEMDUMP_VAR() for the compile time created table to either
automatically create that ID derived from the variable name or you add
an extra argument with the ID.
kmemdump_init()
// Use a simple fixed size array to manage this
// as it avoids all the memory allocation nonsense
// This stuff is neither performance critical nor does allocating
// a few hundred entries create a memory consumption problem
// It consumes probably way less memory than the whole IDR/XARRAY allocation
// string duplication logic consumes text and data space.
kmemdump_entries = kcalloc(NR_ENTRIES, sizeof(*kmemdump_entries), GFP_KERNEL);
kmemdump_register(void *ptr, size_t size, enum kmemdump_uids uid)
{
guard(entry_mutex);
entry = kmemdump_find_empty_slot();
if (!entry)
return;
entry->ptr = ptr;
entry->size = size;
entry->uid = uid;
// Make this unconditional by providing a dummy backend
// implementation. If the backend changes re-register all
// entries with the new backend and be done with it.
backend->register(entry);
}
kmemdump_unregister(void *ptr)
{
guard(entry_mutex);
entry = find_entry(ptr);
if (entry) {
backend->unregister(entry);
memset(entry, 0, sizeof(*entry);
}
}
You get the idea.
Coming back to the registration at the call site itself.
struct foo = kmalloc(....);
if (!foo)
return;
kmemdump_register(foo, sizeof(*foo), KMEMDUMP_ID_FOO);
That's a code duplication shitshow. You can wrap that into:
struct foo *foo = kmemdump_alloc(foo, KMEMDUMP_ID_FOO, kmalloc, ...);
#define kmemdump_alloc(var, id, fn, ...) \
({ \
void *__p = fn(##__VA_ARGS__); \
\
if (__p) \
kmemdump_register(__p, sizeof(*var), id); \
__p;
})
or something daft like that. And provide the matching magic for the free
side.
Thoughts?
Thanks,
tglx
On 6/14/25 00:10, Thomas Gleixner wrote:
> On Fri, Jun 13 2025 at 17:33, Eugen Hristev wrote:
>> On 5/7/25 13:27, Eugen Hristev wrote:
>>>> Let KMEMDUMP_VAR() store the size and the address of 'nr_irqs' in a
>>>> kmemdump specific section and then kmemdump can just walk that section
>>>> and dump stuff. No magic register functions and no extra storage
>>>> management for static/global variables.
>>>>
>>>> No?
>>>
>>> Thank you very much for your review ! I will try it out.
>>
>> I have tried this way and it's much cleaner ! thanks for the
>> suggestion.
>
> Welcome.
>
>> The thing that I am trying to figure out now is how to do something
>> similar for a dynamically allocated memory, e.g.
>> void *p = kmalloc(...);
>> and then I can annotate `p` itself, it's address and size, but what I
>> would also want to so dump the whole memory region pointed out by p. and
>> that area address and size cannot be figured out at compile time hence I
>> can't instantiate a struct inside the dedicated section for it.
>> Any suggestion on how to make that better ? Or just keep the function
>> call to register the area into kmemdump ?
>
> Right. For dynamically allocated memory there is obviously no compile
> time magic possible.
>
> But I think you can simplify the registration for dynamically allocated
> memory significantly.
>
> struct kmemdump_entry {
> void *ptr;
> size_t size;
> enum kmemdump_uids uid;
> };
>
> You use that layout for the compile time table and the runtime
> registrations.
>
> I intentionally used an UID as that avoids string allocation and all of
> the related nonsense. Mapping UID to a string is a post processing
> problem and really does not need to be done in the kernel. The 8
> character strings are horribly limited and a simple 4 byte unique id is
> achieving the same and saving space.
>
> Just stick the IDs into include/linux/kmemdump_ids.h and expose the
> content for the post processing machinery.
>
> So you want KMEMDUMP_VAR() for the compile time created table to either
> automatically create that ID derived from the variable name or you add
> an extra argument with the ID.
First of all, thank you very much for taking the time to think about this !
In KMEMDUMP_VAR, I can use __UNIQUE_ID to derive something unique from
the variable name for the table entry.
The only problem with having something like
#define KMEMDUMP_VAR(sym) \
static struct entry __UNIQUE_ID(kmemdump_entry_##sym) ...
is when calling it with e.g. `init_mm.pgd` which will make the `.`
inside the name and that can't happen.
So I have to figure a way to remove unwanted chars or pass a name to the
macro.
I cannot do something like
static void * ptr = &init_mm.pgd;
and then
KMEMDUMP_VAR(ptr)
because ptr's dereferencing can't happen at compile time to add it's
value into the table entry.
>
> kmemdump_init()
> // Use a simple fixed size array to manage this
> // as it avoids all the memory allocation nonsense
> // This stuff is neither performance critical nor does allocating
> // a few hundred entries create a memory consumption problem
> // It consumes probably way less memory than the whole IDR/XARRAY allocation
> // string duplication logic consumes text and data space.
> kmemdump_entries = kcalloc(NR_ENTRIES, sizeof(*kmemdump_entries), GFP_KERNEL);
>
> kmemdump_register(void *ptr, size_t size, enum kmemdump_uids uid)
> {
> guard(entry_mutex);
>
> entry = kmemdump_find_empty_slot();
> if (!entry)
> return;
>
> entry->ptr = ptr;
> entry->size = size;
> entry->uid = uid;
>
> // Make this unconditional by providing a dummy backend
> // implementation. If the backend changes re-register all
> // entries with the new backend and be done with it.
> backend->register(entry);
> }
>
> kmemdump_unregister(void *ptr)
> {
> guard(entry_mutex);
> entry = find_entry(ptr);
> if (entry) {
> backend->unregister(entry);
> memset(entry, 0, sizeof(*entry);
> }
> }
>
> You get the idea.
>
> Coming back to the registration at the call site itself.
>
> struct foo = kmalloc(....);
>
> if (!foo)
> return;
>
> kmemdump_register(foo, sizeof(*foo), KMEMDUMP_ID_FOO);
>
> That's a code duplication shitshow. You can wrap that into:
>
> struct foo *foo = kmemdump_alloc(foo, KMEMDUMP_ID_FOO, kmalloc, ...);
>
> #define kmemdump_alloc(var, id, fn, ...) \
> ({ \
> void *__p = fn(##__VA_ARGS__); \
> \
> if (__p) \
> kmemdump_register(__p, sizeof(*var), id); \
> __p;
> })
>
I was thinking into a new variant of kmalloc, like e.g. kdmalloc() which
would be a wrapper over kmalloc and also register the region into
kmemdump like you are suggesting.
It would be like a `dumpable` kmalloc'ed memory.
And it could take an optional ID , if missing, it could generate one.
However this would mean yet another k*malloc friend, and it would
default to usual kmalloc if CONFIG_KMEMDUMP=n .
I am unsure whether this would be welcome by the community
Let me know what you think.
Thanks again !
Eugen
> or something daft like that. And provide the matching magic for the free
> side.
>
> Thoughts?
>
> Thanks,
>
> tglx
On Mon, Jun 16 2025 at 13:12, Eugen Hristev wrote:
> On 6/14/25 00:10, Thomas Gleixner wrote:
> #define KMEMDUMP_VAR(sym) \
> static struct entry __UNIQUE_ID(kmemdump_entry_##sym) ...
>
> is when calling it with e.g. `init_mm.pgd` which will make the `.`
> inside the name and that can't happen.
> So I have to figure a way to remove unwanted chars or pass a name to the
> macro.
You can do:
KMEMDUMP_VAR_MEMBER(init_mm, pgd);
and concatenate with a '.' for the symbol
and a '_' for the ID.
or simply
KMEMDUMP_VAR_ID(init_mm.pgg, INIT_MM_PGD);
>> #define kmemdump_alloc(var, id, fn, ...) \
>> ({ \
>> void *__p = fn(##__VA_ARGS__); \
>> \
>> if (__p) \
>> kmemdump_register(__p, sizeof(*var), id); \
>> __p;
>> })
>>
>
> I was thinking into a new variant of kmalloc, like e.g. kdmalloc() which
> would be a wrapper over kmalloc and also register the region into
> kmemdump like you are suggesting.
> It would be like a `dumpable` kmalloc'ed memory.
> And it could take an optional ID , if missing, it could generate one.
>
> However this would mean yet another k*malloc friend, and it would
> default to usual kmalloc if CONFIG_KMEMDUMP=n .
> I am unsure whether this would be welcome by the community
That's definitely more welcome than copying boilerplate code all over
the place. And if you do it the way I suggested, then you only have
_one_ macro for alloc and _one_ for free because you hand in the
allocation function with all of its arguments instead of creating an
ever increasing zoo of dedicated variants. But the MM people might have
different opinions.
Thanks,
tglx
© 2016 - 2026 Red Hat, Inc.