Re: [RFC][PATCH 09/14] genirq: add irq_kmemdump_register

[Thomas Gleixner <tglx@linutronix.de>]

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