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

[Mike Rapoport <rppt@kernel.org>]

On Tue, Feb 17, 2026 at 04:45:07PM -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 (like unused memory "set" using
> "mem=") nor in the kernel log (like the "crashkernel" parameter does).
> 
> Add here a new file under memblock debugfs showing properly set memory
> reservations, with name, address and size as passed to "reserve_mem".
> 
> Notice this addition makes the memblock folder *always available* under
> debugfs, regardless of ARCH_KEEP_MEMBLOCK and even if there is no
> "reserve_mem=" setting in the command-line.
> 
> 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>
> ---
>  mm/memblock.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 2d2646f7a120..58439de0a59b 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -18,6 +18,10 @@
>  #include <linux/memblock.h>
>  #include <linux/mutex.h>
>  
> +#ifdef CONFIG_DEBUG_FS
> +#include <linux/string_helpers.h>
> +#endif

No need to ifdef this, a small additional header is not a big deal and it's
anyway included in seq_file.h.

> +
>  #ifdef CONFIG_KEXEC_HANDOVER
>  #include <linux/libfdt.h>
>  #include <linux/kexec_handover.h>
> @@ -2711,7 +2715,7 @@ 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
>  static const char * const flagname[] = {

Flag names only needed when ARCH_KEEP_MEMBLOCK is set ... 
>  	[ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
>  	[ilog2(MEMBLOCK_MIRROR)] = "MIRROR",
> @@ -2722,7 +2726,8 @@ static const char * const flagname[] = {
>  	[ilog2(MEMBLOCK_KHO_SCRATCH)] = "KHO_SCRATCH",
>  };
>  
> -static int memblock_debug_show(struct seq_file *m, void *private)
> +#ifdef CONFIG_ARCH_KEEP_MEMBLOCK

... so this ifdef should go before the flagname declaration.

> +static void memblock_debugfs_files(struct seq_file *m)
>  {
>  	struct memblock_type *type = m->private;
>  	struct memblock_region *reg;
> @@ -2754,6 +2759,30 @@ static int memblock_debug_show(struct seq_file *m, void *private)
>  			seq_printf(m, "%s\n", "NONE");
>  		}
>  	}
> +}
> +#else
> +static void memblock_debugfs_files(struct seq_file *m) {}
> +#endif /* CONFIG_ARCH_KEEP_MEMBLOCK */
> +
> +static int memblock_debug_show(struct seq_file *m, void *private)
> +{
> +	if (m->private == &reserved_mem_table[0]) {
> +		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, 16);
> +			string_get_size((u64)(map->size), 1, STRING_UNITS_2, txtsz, 16);

phys_addr_t should be casted automatically to u64 IMO.
And please, no magic numbers.

> +			seq_printf(m, "%s\t\t%pa\t(%s)\n",
> +				map->name, &map->start, txtsz);

Let's not expose the physical address, name and size should be enough to
see which reserve_mem allocations succeeded.

> +		}

You can define "reserve_mem" attribute separately, and leave the existing
memblock_debug_show() as it was.

> +	} else
> +		memblock_debugfs_files(m);
> +
>  	return 0;
>  }
>  DEFINE_SHOW_ATTRIBUTE(memblock_debug);
> @@ -2762,6 +2791,9 @@ static int __init memblock_init_debugfs(void)
>  {
>  	struct dentry *root = debugfs_create_dir("memblock", NULL);

No need to create memblock directory in debugfs if there's nothing to show.
Could be something like

	if (i!(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) || reserved_mem_count))
		return;

	root = debugfs_create_dir("memblock", NULL);
  
> +	debugfs_create_file("reserve_mem_param", 0444, root,
> +			    &reserved_mem_table[0], &memblock_debug_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 */
>  	return 0;
>  }
>  __initcall(memblock_init_debugfs);
> -- 
> 2.50.1
> 
> 

-- 
Sincerely yours,
Mike.