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

[Mike Rapoport <rppt@kernel.org>]

On Sun, Feb 22, 2026 at 10:58:27AM -0300, Guilherme G. Piccoli wrote:
> 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?
 
Please resend both patches together. 
 
> 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.
 
sizeof(txtsz) should work :) 
 
> > [...] 
> > 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?

Yes, something like

static int memblock_reserve_mem_show(struct seq_file *m, void *private)
{
	...
}
DEFINE_SHOW_ATTRIBUTE(memblock_reserve_mem);
 
> 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.

Yes, that's what I meant.

> Cheers,
> Guilherme

-- 
Sincerely yours,
Mike.