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
