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

Mike Rapoport
2026-02-22T19:19:17Z

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.