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
