Re: [PATCH V2 2/2] mm/memblock: Add reserve_mem debugfs info

[Mike Rapoport <rppt@kernel.org>]

Hi Guilherme,

On Wed, Mar 04, 2026 at 05:14:11PM -0300, Guilherme G. Piccoli wrote:
> When using the "reserve_mem" parameter, users aim at having an
> area that (hopefully) persists across boots, so pstore infrastructure
> (like ramoops module) can make use of that to save oops/ftrace logs,
> for example.
> 
> There is no easy way to determine if this kernel parameter is properly
> set though; the kernel doesn't show information about this memory in
> memblock debugfs, neither in /proc/iomem nor dmesg. This is a relevant
> information for tools like kdumpst[0], to determine if it's reliable
> to use the reserved area as ramoops persistent storage; checking only
> /proc/cmdline is not sufficient as it doesn't tell if the reservation
> effectively succeeded or not.
> 
> Add here a new file under memblock debugfs showing properly set memory
> reservations, with name and size as passed to "reserve_mem". Notice that
> if no "reserve_mem=" is passed on command-line or if the reservation
> attempts fail, the file is not created.
> 
> [0] https://aur.archlinux.org/packages/kdumpst
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
> 
> 
> Thanks a lot for the suggestions Mike! I'm not sure if you would
> prefer a Co-Developed-by or Suggested-by instead of CC, you helped
> a lot improving the code. Lemme know and either I can re-submit
> (with potential other changes) or even, you can change while merging.
> 
> V2: (all suggestions by Mike Rapoport)
> 
> - Commit message (showing use case);
> - Drop ifdef on include "string_helpers.h";
> - Don't show the address of reserve_mem, only name and size;
> - Fixed flag names inside ARCH_KEEP_MEMBLOCK ifdef;
> - Make use of its own show_attribute instead of refactoring
>   the memblock one;
> - Use sizeof() instead of magical numbers for the size;
> - Don't show memblock directory if no reserve_mem succeeded
>   and ARCH_KEEP_MEMBLOCK isn't defined (keeping current behavior).
> 
> 
>  mm/memblock.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 2d2646f7a120..d816796ab919 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -17,6 +17,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/memblock.h>
>  #include <linux/mutex.h>
> +#include <linux/string_helpers.h>

This will break tests in tools/testing/memblock, please add a stub there.
 
>  #ifdef CONFIG_KEXEC_HANDOVER
>  #include <linux/libfdt.h>
> @@ -2711,7 +2712,8 @@ static int __init reserve_mem(char *p)
>  }
>  __setup("reserve_mem=", reserve_mem);
>  
> -#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_ARCH_KEEP_MEMBLOCK)
> +#ifdef CONFIG_DEBUG_FS
> +#ifdef CONFIG_ARCH_KEEP_MEMBLOCK
>  static const char * const flagname[] = {
>  	[ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
>  	[ilog2(MEMBLOCK_MIRROR)] = "MIRROR",
> @@ -2758,10 +2760,40 @@ static int memblock_debug_show(struct seq_file *m, void *private)
>  }
>  DEFINE_SHOW_ATTRIBUTE(memblock_debug);
>  
> +#endif /* CONFIG_ARCH_KEEP_MEMBLOCK */
> +
> +static int memblock_reserve_mem_show(struct seq_file *m, void *private)
> +{
> +	struct reserve_mem_table *map;
> +	char txtsz[16];
> +
> +	for (int i = 0; i < reserved_mem_count; i++) {
> +		map = &reserved_mem_table[i];
> +		if (!map->size)
> +			continue;
> +
> +		memset(txtsz, 0, sizeof(txtsz));
> +		string_get_size(map->size, 1, STRING_UNITS_2, txtsz, sizeof(txtsz));
> +		seq_printf(m, "%s\t\t(%s)\n", map->name, txtsz);
> +	}
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(memblock_reserve_mem);
> +
>  static int __init memblock_init_debugfs(void)
>  {
> -	struct dentry *root = debugfs_create_dir("memblock", NULL);
> +	struct dentry *root;
>  
> +	if (!(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) || reserved_mem_count))
> +		return 0;
> +
> +	root = debugfs_create_dir("memblock", NULL);
> +
> +	if (reserved_mem_count)
> +		debugfs_create_file("reserve_mem_param", 0444, root, NULL,
> +				    &memblock_reserve_mem_fops);
> +#ifdef CONFIG_ARCH_KEEP_MEMBLOCK
>  	debugfs_create_file("memory", 0444, root,
>  			    &memblock.memory, &memblock_debug_fops);
>  	debugfs_create_file("reserved", 0444, root,
> @@ -2771,6 +2803,7 @@ static int __init memblock_init_debugfs(void)
>  			    &memblock_debug_fops);
>  #endif
>  
> +#endif /* CONFIG_ARCH_KEEP_MEMBLOCK */

It becomes too #ifdef'y :(
Let's put the creation of these files into a helper and add an empty stub
when ARCH_KEEP_MEMBLOCK=n.

>  	return 0;
>  }
>  __initcall(memblock_init_debugfs);
> -- 
> 2.50.1
> 

-- 
Sincerely yours,
Mike.