[PATCH 0/2] Some small improvements to reserve_mem

[PATCH 0/2] Some small improvements to reserve_mem

[Guilherme G. Piccoli]

Currently the "reserve_mem" parsing is lacking information both
if it works or not. It can fail in many ways, so I'm adding some
messages to help users determine failure.

At the same time, if it succeeds, the only place I can "see" it
is in the accounting of reserved memory, as the following kernel message:

[0.086881] Memory: 3958852K/4189472K available (19671K kernel code, 2893K rwdata, 9724K rodata, 4340K init, 5040K bss, 220244K reserved, 0K cma-reserved)

Since "crashkernel=" reservations are shown in both kernel log and /proc/iomem
and other unused memory buffers appear in /proc/iomem as "RAM buffer" entries,
I've added hereby a debugfs entry for "reserve_mem". Though I didn't love the
implementation, I think it's not so hideous so decided to send, please lemme
know what you think and if we should improve code or even discard the idea heheh

Notice that, with this change, the memblock debugfs folder ends-up showing
always, no matter if we have ARCH_KEEP_MEMBLOCK set or if reserve_mem is set.
Thanks in advance for reviews/comments!


Guilherme G. Piccoli (2):
  mm/memblock: Print out errors on reserve_mem parser
  mm/memblock: Add reserve_mem debugfs info

 mm/memblock.c | 65 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 54 insertions(+), 11 deletions(-)

-- 
2.50.1

[PATCH 1/2] mm/memblock: Print out errors on reserve_mem parser

[Guilherme G. Piccoli]

The parsing of kernel parameter "reserve_mem=" is subject to
multiple failures, like duplicate naming, malformed expression
or even lack of available memory. Right now, all of these fail
silently. Let's add some messages so the kernel log can provide
useful information in case of failures.

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 | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index b3ddfdec7a80..2d2646f7a120 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -2642,23 +2642,25 @@ static int __init reserve_mem(char *p)
 	int len;
 
 	if (!p)
-		return -EINVAL;
+		goto err_param;
 
 	/* Check if there's room for more reserved memory */
-	if (reserved_mem_count >= RESERVE_MEM_MAX_ENTRIES)
+	if (reserved_mem_count >= RESERVE_MEM_MAX_ENTRIES) {
+		pr_err("reserve_mem: no more room for reserved memory\n");
 		return -EBUSY;
+	}
 
 	oldp = p;
 	size = memparse(p, &p);
 	if (!size || p == oldp)
-		return -EINVAL;
+		goto err_param;
 
 	if (*p != ':')
-		return -EINVAL;
+		goto err_param;
 
 	align = memparse(p+1, &p);
 	if (*p != ':')
-		return -EINVAL;
+		goto err_param;
 
 	/*
 	 * memblock_phys_alloc() doesn't like a zero size align,
@@ -2672,7 +2674,7 @@ static int __init reserve_mem(char *p)
 
 	/* name needs to have length but not too big */
 	if (!len || len >= RESERVE_MEM_NAME_SIZE)
-		return -EINVAL;
+		goto err_param;
 
 	/* Make sure that name has text */
 	for (p = name; *p; p++) {
@@ -2680,11 +2682,13 @@ static int __init reserve_mem(char *p)
 			break;
 	}
 	if (!*p)
-		return -EINVAL;
+		goto err_param;
 
 	/* Make sure the name is not already used */
-	if (reserve_mem_find_by_name(name, &start, &tmp))
+	if (reserve_mem_find_by_name(name, &start, &tmp)) {
+		pr_err("reserve_mem: name \"%s\" was already used\n", name);
 		return -EBUSY;
+	}
 
 	/* Pick previous allocations up from KHO if available */
 	if (reserve_mem_kho_revive(name, size, align))
@@ -2692,12 +2696,18 @@ static int __init reserve_mem(char *p)
 
 	/* TODO: Allocation must be outside of scratch region */
 	start = memblock_phys_alloc(size, align);
-	if (!start)
+	if (!start) {
+		pr_err("reserve_mem: memblock allocation failed\n");
 		return -ENOMEM;
+	}
 
 	reserved_mem_add(start, size, name);
 
 	return 1;
+err_param:
+	pr_err("reserve_mem: empty or malformed parameter\n");
+	return -EINVAL;
+
 }
 __setup("reserve_mem=", reserve_mem);
 
-- 
2.50.1

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

[Guilherme G. Piccoli]

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
+
 #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[] = {
 	[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
+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);
+			seq_printf(m, "%s\t\t%pa\t(%s)\n",
+				map->name, &map->start, txtsz);
+		}
+	} 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);
 
+	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

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

[kernel test robot]

Hi Guilherme,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rppt-memblock/for-next]
[also build test WARNING on rppt-memblock/fixes akpm-mm/mm-everything linus/master v6.19 next-20260217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Guilherme-G-Piccoli/mm-memblock-Print-out-errors-on-reserve_mem-parser/20260218-035935
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rppt/memblock.git for-next
patch link:    https://lore.kernel.org/r/20260217195816.861684-3-gpiccoli%40igalia.com
patch subject: [PATCH 2/2] mm/memblock: Add reserve_mem debugfs info
config: xtensa-randconfig-r071-20260218 (https://download.01.org/0day-ci/archive/20260218/202602180841.dpP5OelN-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 11.5.0
smatch version: v0.5.0-8994-gd50c5a4c
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260218/202602180841.dpP5OelN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602180841.dpP5OelN-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> mm/memblock.c:2721:27: warning: 'flagname' defined but not used [-Wunused-const-variable=]
    2721 | static const char * const flagname[] = {
         |                           ^~~~~~~~


vim +/flagname +2721 mm/memblock.c

1e4c64b71c9bf2 Steven Rostedt (Google    2024-06-13  2719) 
54091ffecefa11 Guilherme G. Piccoli      2026-02-17  2720  #ifdef CONFIG_DEBUG_FS
493f349e38d022 Yuwei Guan                2023-05-19 @2721  static const char * const flagname[] = {
493f349e38d022 Yuwei Guan                2023-05-19  2722  	[ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
493f349e38d022 Yuwei Guan                2023-05-19  2723  	[ilog2(MEMBLOCK_MIRROR)] = "MIRROR",
493f349e38d022 Yuwei Guan                2023-05-19  2724  	[ilog2(MEMBLOCK_NOMAP)] = "NOMAP",
493f349e38d022 Yuwei Guan                2023-05-19  2725  	[ilog2(MEMBLOCK_DRIVER_MANAGED)] = "DRV_MNG",
4f155af0ae4464 Anshuman Khandual         2024-02-09  2726  	[ilog2(MEMBLOCK_RSRV_NOINIT)] = "RSV_NIT",
4c78cc596bb8d3 Mike Rapoport (Microsoft  2025-05-09  2727) 	[ilog2(MEMBLOCK_RSRV_KERN)] = "RSV_KERN",
d59f43b5748092 Alexander Graf            2025-05-09  2728  	[ilog2(MEMBLOCK_KHO_SCRATCH)] = "KHO_SCRATCH",
493f349e38d022 Yuwei Guan                2023-05-19  2729  };
6d03b885f0926a Benjamin Herrenschmidt    2010-07-06  2730  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Guilherme,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rppt-memblock/for-next]
[also build test WARNING on rppt-memblock/fixes akpm-mm/mm-everything linus/master v6.19 next-20260217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Guilherme-G-Piccoli/mm-memblock-Print-out-errors-on-reserve_mem-parser/20260218-035935
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rppt/memblock.git for-next
patch link:    https://lore.kernel.org/r/20260217195816.861684-3-gpiccoli%40igalia.com
patch subject: [PATCH 2/2] mm/memblock: Add reserve_mem debugfs info
config: s390-randconfig-002-20260218 (https://download.01.org/0day-ci/archive/20260218/202602180820.Vi6LlvfF-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project e86750b29fa0ff207cd43213d66dabe565417638)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260218/202602180820.Vi6LlvfF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602180820.Vi6LlvfF-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> mm/memblock.c:2721:27: warning: unused variable 'flagname' [-Wunused-const-variable]
    2721 | static const char * const flagname[] = {
         |                           ^~~~~~~~
   1 warning generated.


vim +/flagname +2721 mm/memblock.c

1e4c64b71c9bf2 Steven Rostedt (Google    2024-06-13  2719) 
54091ffecefa11 Guilherme G. Piccoli      2026-02-17  2720  #ifdef CONFIG_DEBUG_FS
493f349e38d022 Yuwei Guan                2023-05-19 @2721  static const char * const flagname[] = {
493f349e38d022 Yuwei Guan                2023-05-19  2722  	[ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
493f349e38d022 Yuwei Guan                2023-05-19  2723  	[ilog2(MEMBLOCK_MIRROR)] = "MIRROR",
493f349e38d022 Yuwei Guan                2023-05-19  2724  	[ilog2(MEMBLOCK_NOMAP)] = "NOMAP",
493f349e38d022 Yuwei Guan                2023-05-19  2725  	[ilog2(MEMBLOCK_DRIVER_MANAGED)] = "DRV_MNG",
4f155af0ae4464 Anshuman Khandual         2024-02-09  2726  	[ilog2(MEMBLOCK_RSRV_NOINIT)] = "RSV_NIT",
4c78cc596bb8d3 Mike Rapoport (Microsoft  2025-05-09  2727) 	[ilog2(MEMBLOCK_RSRV_KERN)] = "RSV_KERN",
d59f43b5748092 Alexander Graf            2025-05-09  2728  	[ilog2(MEMBLOCK_KHO_SCRATCH)] = "KHO_SCRATCH",
493f349e38d022 Yuwei Guan                2023-05-19  2729  };
6d03b885f0926a Benjamin Herrenschmidt    2010-07-06  2730  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 0/2] Some small improvements to reserve_mem

[Mike Rapoport]

Hi Guilherme,

On Tue, Feb 17, 2026 at 04:45:05PM -0300, Guilherme G. Piccoli wrote:
> Currently the "reserve_mem" parsing is lacking information both
> if it works or not. It can fail in many ways, so I'm adding some
> messages to help users determine failure.

Yep, these are useful. 

> At the same time, if it succeeds, the only place I can "see" it
> is in the accounting of reserved memory, as the following kernel message:
> 
> [0.086881] Memory: 3958852K/4189472K available (19671K kernel code, 2893K rwdata, 9724K rodata, 4340K init, 5040K bss, 220244K reserved, 0K cma-reserved)
> 
> Since "crashkernel=" reservations are shown in both kernel log and /proc/iomem
> and other unused memory buffers appear in /proc/iomem as "RAM buffer" entries,
> I've added hereby a debugfs entry for "reserve_mem". Though I didn't love the
> implementation, I think it's not so hideous so decided to send, please lemme
> know what you think and if we should improve code or even discard the idea heheh

crashkernel reservations are part of the ABI and kexec tools parse them in
userspace, so they should be exposed. reserve_mem consumers are in the
kernel, so if the reservation succeeds there's no real need for userspace
to see where are the reserve_mem regions.

I don't generally object showing them in debugfs, but the changelog for
debugfs addition should give more details why they would be useful rather
than just say that unlike crashkernel they don't show up anywhere.
 
> Notice that, with this change, the memblock debugfs folder ends-up showing
> always, no matter if we have ARCH_KEEP_MEMBLOCK set or if reserve_mem is set.
> Thanks in advance for reviews/comments!
> 
> 
> Guilherme G. Piccoli (2):
>   mm/memblock: Print out errors on reserve_mem parser
>   mm/memblock: Add reserve_mem debugfs info
> 
>  mm/memblock.c | 65 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 54 insertions(+), 11 deletions(-)
> 
> -- 
> 2.50.1
> 
> 

-- 
Sincerely yours,
Mike.

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

[Mike Rapoport]

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.

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

[Guilherme G. Piccoli]

Hi Mike, thanks a bunch for your review! Good ideas, I'll change the
implementation to follow the suggestions and improve the changelog, as
you suggested. I have some questions though, that I will comment inline,
below.

First of all, what should we do regarding patch 1? Should I resubmit as
part of V2, even with no changes - or pick it now and I only submit
patch 2 as V2, with changes?



On 21/02/2026 05:52, Mike Rapoport wrote:
> [...]
>> +			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.

Specifically here, by magic number you mean my choice of 16, right? What
do you suggest me to pick? It's the length of the string carrying the
size of reserved_mem, some number must be selected for this
length...lemme know WDYT.



> [...] 
> 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);

Very good suggestions here, but just let me clarify: so I could continue
showing the "reserved_mem_param" inside the "<debugfs>/memblock" folder,
just using a different function for that attribute?

I understood that, based on your (good) suggestion to hide the memblock
folder if ARCH_KEEP_MEMBLOCK is not defined and there is no reserved_mem
set ... just want to confirm to follow-up the implementation.
Cheers,


Guilherme