Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs

[Mike Rapoport <rppt@xxxxxxxxxx>]

On Sun, May 28, 2023 at 10:38:03PM +0800, Yuwei Guan wrote:
> Mike Rapoport <rppt@xxxxxxxxxx> 于2023年5月27日周六 18:37写道：
> >
> > Hi Yuwei,
> >
> > On Fri, May 19, 2023 at 06:53:21PM +0800, Yuwei Guan wrote:
> >
> > > Signed-off-by: Yuwei Guan <ssawgyw@xxxxxxxxx>
> > > ---
> > > v4:
> > > - show string value for each memblock flag
> > > ---
> > >  mm/memblock.c | 24 ++++++++++++++++++++++--
> > >  1 file changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 511d4783dcf1..10d0ddbeebc1 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -2136,12 +2136,19 @@ void __init memblock_free_all(void)
> > >  }
> > >
> > >  #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_ARCH_KEEP_MEMBLOCK)
> > > +static const char * const flagname[] = {
> > > +     [ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
> > > +     [ilog2(MEMBLOCK_MIRROR)] = "MIRROR",
> > > +     [ilog2(MEMBLOCK_NOMAP)] = "NOMAP",
> > > +     [ilog2(MEMBLOCK_DRIVER_MANAGED)] = "DRV_MNG",
> > > +};
> > >
> > >  static int memblock_debug_show(struct seq_file *m, void *private)
> > >  {
> > >       struct memblock_type *type = m->private;
> > >       struct memblock_region *reg;
> > > -     int i;
> > > +     int i, j;
> > > +     unsigned int count = ARRAY_SIZE(flagname);
> > >       phys_addr_t end;
> > >
> > >       for (i = 0; i < type->cnt; i++) {
> > > @@ -2149,7 +2156,20 @@ static int memblock_debug_show(struct seq_file *m, void *private)
> > >               end = reg->base + reg->size - 1;
> > >
> > >               seq_printf(m, "%4d: ", i);
> > > -             seq_printf(m, "%pa..%pa\n", &reg->base, &end);
> > > +             seq_printf(m, "%pa..%pa ", &reg->base, &end);
> > > +             seq_printf(m, "%4d ", memblock_get_region_node(reg));
> >
> > As Kefeng mentioned, the node id for reserved regions will be wrong, so
> > this needs to be updated so that when reg->nid == MAX_NUMNODES we'll print
> > e.g. 'x'.
> Hi Mike,
> 
> How about print 'x' when reg->nid == MAX_NUMNODES, base on this patch,
> https://lore.kernel.org/linux-mm/44ce007d-8049-1cc9-7e2e-4ccb51a2867d@xxxxxxxxxx/T/#m6d214d4ea8912b64f5efc9c3f51ae71aa057d1f1

This is fine, can you please send a formal patch?
 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index c5c80d9bcea3..3d449aaba052 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2169,17 +2169,21 @@ static int memblock_debug_show(struct seq_file
> *m, void *private)
>  {
>   struct memblock_type *type = m->private;
>   struct memblock_region *reg;
> - int i, j;
> + int i, j, nid;
>   unsigned int count = ARRAY_SIZE(flagname);
>   phys_addr_t end;
> 
>   for (i = 0; i < type->cnt; i++) {
>   reg = &type->regions[i];
>   end = reg->base + reg->size - 1;
> + nid = memblock_get_region_node(reg);
> 
>   seq_printf(m, "%4d: ", i);
>   seq_printf(m, "%pa..%pa ", &reg->base, &end);
> - seq_printf(m, "%4d ", memblock_get_region_node(reg));
> + if (nid != MAX_NUMNODES)
> + seq_printf(m, "%4d ", nid);
> + else
> + seq_printf(m, "%4c ", 'x');
>   if (reg->flags) {
>   for (j = 0; j < count; j++) {
>   if (reg->flags & (1U << j)) {
> 
> >
> > > +             if (reg->flags) {
> > > +                     for (j = 0; j < count; j++) {
> > > +                             if (reg->flags & (1U << j)) {
> > > +                                     seq_printf(m, "%s\n", flagname[j]);
> > > +                                     break;
> > > +                             }
> > > +                     }
> > > +                     if (j == count)
> > > +                             seq_printf(m, "%s\n", "UNKNOWN");
> > > +             } else {
> > > +                     seq_printf(m, "%s\n", "NONE");
> > > +             }
> > >       }
> > >       return 0;
> > >  }
> > > --
> > > 2.34.1
> > >
> >
> > --
> > Sincerely yours,
> > Mike.

-- 
Sincerely yours,
Mike.