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>
 
 #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 */
 	return 0;
 }
 __initcall(memblock_init_debugfs);
-- 
2.50.1

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.

On Wed,  4 Mar 2026 17:14:11 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> 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.

Asking out of curiosity.  The previous patch has added the error log for
failure cases.  Could checking the kernel log to see if it was failed be an
option?

I think this debugfs approach is easier to check for the user space, though.
If that is the reason of this patch, adding the clarity would be nice for a
theoretical case that debugfs cannot be mounted.

> 
> 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>
>  
>  #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;

One trivial comment.  I'd slightly prefer having one less parentheses level as
a tradeoff of having one more exclamation mark, e.g.,

    if (!IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) && !reserved_mem_count)

> +
> +	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 */

So, we get two level of nested ifdef...  I'm wondering if returning earlier
when CONFIG_ARCH_KEEP_MEMBLOCK is undefined is easier to read.  E.g.,

	#ifndef CONFIG_ARCH_KEEP_MEMBLOCK
		return 0;
	#endif

		debugfs_create_file("memory", 0444, root,
	[...]

>  	return 0;

Very trivial comment.  Why don't you keep the original blank line above the
return statemtnt?

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


Thanks,
SJ

Hi SJ and Mike, thanks for both your review! I'll respond the 2 messages
here.

First regarding Mike's concern from the other email:

>> +#include <linux/string_helpers.h>
>This will break tests in tools/testing/memblock, please add a stub there.

Sure, will do it!

On 04/03/2026 22:44, SeongJae Park wrote:
>[...]
>> 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.
> 
> Asking out of curiosity.  The previous patch has added the error log for
> failure cases.  Could checking the kernel log to see if it was failed be an
> option?
> 
> I think this debugfs approach is easier to check for the user space, though.
> If that is the reason of this patch, adding the clarity would be nice for a
> theoretical case that debugfs cannot be mounted.
>

Exactly that! Not only debugfs way is more convenient for checking that,
but it feels a bit weird for me to assume the reservation worked by
checking for errors and if none, all fine. It's valid, but as a matter
of taste, I prefer checking the file. And not only that: imagine we have
more than one setting in the cmdline, for other usages like ftrace.
Easier to have the debugfs showing the succeeding ones, by name and
size, ready to be used heheh

Do you have a suggestion on how should I mention this in the commit
message? Is that discussion enough maybe, for future reference? I feel
mentioning that userspace will use the information from the file seems
enough for a commit message; but I'm totally open for your suggestions
on how to improve the wording here


>> +	if (!(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) || reserved_mem_count))
>> +		return 0;
> 
> One trivial comment.  I'd slightly prefer having one less parentheses level as
> a tradeoff of having one more exclamation mark, e.g.,
> 
>     if (!IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) && !reserved_mem_count)
> 

Good! Unless Mike is against that, I can easily change it.


> 
> So, we get two level of nested ifdef...  I'm wondering if returning earlier
> when CONFIG_ARCH_KEEP_MEMBLOCK is undefined is easier to read.  E.g.,
> 
> 	#ifndef CONFIG_ARCH_KEEP_MEMBLOCK
> 		return 0;
> 	#endif
> 
> 		debugfs_create_file("memory", 0444, root,
> 	[...]
> 
>>  	return 0;
> 
> Very trivial comment.  Why don't you keep the original blank line above the
> return statemtnt?
> 

About the ifdefs, as per Mike's comment, I'll rework it in a helper. And
the blank like, I have no answer heh
Likely a braino? I can certainly keep it.

Thanks again you two for the suggestions,


Guilherme

On Fri, 13 Mar 2026 18:09:10 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:

> Hi SJ and Mike, thanks for both your review! I'll respond the 2 messages
> here.
> 
> First regarding Mike's concern from the other email:
> 
> >> +#include <linux/string_helpers.h>
> >This will break tests in tools/testing/memblock, please add a stub there.
> 
> Sure, will do it!
> 
> On 04/03/2026 22:44, SeongJae Park wrote:
> >[...]
> >> 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.
> > 
> > Asking out of curiosity.  The previous patch has added the error log for
> > failure cases.  Could checking the kernel log to see if it was failed be an
> > option?
> > 
> > I think this debugfs approach is easier to check for the user space, though.
> > If that is the reason of this patch, adding the clarity would be nice for a
> > theoretical case that debugfs cannot be mounted.
> >
> 
> Exactly that! Not only debugfs way is more convenient for checking that,
> but it feels a bit weird for me to assume the reservation worked by
> checking for errors and if none, all fine. It's valid, but as a matter
> of taste, I prefer checking the file. And not only that: imagine we have
> more than one setting in the cmdline, for other usages like ftrace.
> Easier to have the debugfs showing the succeeding ones, by name and
> size, ready to be used heheh

Makes sense to me, thank you for kindly clarifying this! :)

> 
> Do you have a suggestion on how should I mention this in the commit
> message? Is that discussion enough maybe, for future reference? I feel
> mentioning that userspace will use the information from the file seems
> enough for a commit message; but I'm totally open for your suggestions
> on how to improve the wording here

The above clarification on this thread is good enough for me.

> 
> 
> >> +	if (!(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) || reserved_mem_count))
> >> +		return 0;
> > 
> > One trivial comment.  I'd slightly prefer having one less parentheses level as
> > a tradeoff of having one more exclamation mark, e.g.,
> > 
> >     if (!IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) && !reserved_mem_count)
> > 
> 
> Good! Unless Mike is against that, I can easily change it.
> 
> 
> > 
> > So, we get two level of nested ifdef...  I'm wondering if returning earlier
> > when CONFIG_ARCH_KEEP_MEMBLOCK is undefined is easier to read.  E.g.,
> > 
> > 	#ifndef CONFIG_ARCH_KEEP_MEMBLOCK
> > 		return 0;
> > 	#endif
> > 
> > 		debugfs_create_file("memory", 0444, root,
> > 	[...]
> > 
> >>  	return 0;
> > 
> > Very trivial comment.  Why don't you keep the original blank line above the
> > return statemtnt?
> > 
> 
> About the ifdefs, as per Mike's comment, I'll rework it in a helper. And
> the blank like, I have no answer heh
> Likely a braino? I can certainly keep it.

Sounds good!

> 
> Thanks again you two for the suggestions,

:)


Thanks,
SJ

[...]