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

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

[Yuwei Guan]

Currently, the memblock debugfs can display the count of memblock_type and
the base and end of the reg. However, when the following scenario occurs,
the information in the existing debugfs cannot make it clear why the
address is not consecutive.

For example,
cat /sys/kernel/debug/memblock/memory
   0: 0x0000000080000000..0x00000000901fffff
   1: 0x0000000090200000..0x00000000905fffff
   2: 0x0000000090600000..0x0000000092ffffff
   3: 0x0000000093000000..0x00000000973fffff
   4: 0x0000000097400000..0x00000000b71fffff
   5: 0x00000000c0000000..0x00000000dfffffff
   6: 0x00000000e2500000..0x00000000f87fffff
   7: 0x00000000f8800000..0x00000000fa7fffff
   8: 0x00000000fa800000..0x00000000fd3effff
   9: 0x00000000fd3f0000..0x00000000fd3fefff
  10: 0x00000000fd3ff000..0x00000000fd7fffff
  11: 0x00000000fd800000..0x00000000fd901fff
  12: 0x00000000fd902000..0x00000000fd909fff
  13: 0x00000000fd90a000..0x00000000fd90bfff
  14: 0x00000000fd90c000..0x00000000ffffffff
  15: 0x0000000880000000..0x0000000affffffff

So we can add flags and nid to this debugfs.

For example,
cat /sys/kernel/debug/memblock/memory
cnt     base..end       flags   nid
0:      0x0000000080000000..0x00000000901fffff  0x0     0x0
1:      0x0000000090200000..0x00000000905fffff  0x4     0x0
2:      0x0000000090600000..0x0000000092ffffff  0x0     0x0
3:      0x0000000093000000..0x00000000973fffff  0x4     0x0
4:      0x0000000097400000..0x00000000b71fffff  0x0     0x0
5:      0x00000000c0000000..0x00000000dfffffff  0x0     0x0
6:      0x00000000e2500000..0x00000000f87fffff  0x0     0x0
7:      0x00000000f8800000..0x00000000fa7fffff  0x4     0x0
8:      0x00000000fa800000..0x00000000fd3effff  0x0     0x0
9:      0x00000000fd3f0000..0x00000000fd3fefff  0x4     0x0
10:     0x00000000fd3ff000..0x00000000fd7fffff  0x0     0x0
11:     0x00000000fd800000..0x00000000fd901fff  0x4     0x0
12:     0x00000000fd902000..0x00000000fd909fff  0x0     0x0
13:     0x00000000fd90a000..0x00000000fd90bfff  0x4     0x0
14:     0x00000000fd90c000..0x00000000ffffffff  0x0     0x0
15:     0x0000000880000000..0x0000000affffffff  0x0     0x0

Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
Reviewed-by: Tarun Sahu <tsahu@linux.ibm.com>
---
 mm/memblock.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 511d4783dcf1..b36fb6b31e0f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -2144,12 +2144,16 @@ static int memblock_debug_show(struct seq_file *m, void *private)
 	int i;
 	phys_addr_t end;
 
+	seq_puts(m, "cnt\tbase..end\tflags\tnid\n");
+
 	for (i = 0; i < type->cnt; i++) {
 		reg = &type->regions[i];
 		end = reg->base + reg->size - 1;
 
-		seq_printf(m, "%4d: ", i);
-		seq_printf(m, "%pa..%pa\n", &reg->base, &end);
+		seq_printf(m, "%d:\t", i);
+		seq_printf(m, "%pa..%pa\t", &reg->base, &end);
+		seq_printf(m, "0x%x\t", reg->flags);
+		seq_printf(m, "0x%x\n", memblock_get_region_node(reg));
 	}
 	return 0;
 }
-- 
2.34.1

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

[Anshuman Khandual]

On 5/17/23 08:27, Yuwei Guan wrote:
> Currently, the memblock debugfs can display the count of memblock_type and
> the base and end of the reg. However, when the following scenario occurs,

scenarios where the memblock flags or nid varies inside a single PA range ?
I guess the commit message description here can be improved to accommodate
such details.

> the information in the existing debugfs cannot make it clear why the
> address is not consecutive.
> 
> For example,
> cat /sys/kernel/debug/memblock/memory
>    0: 0x0000000080000000..0x00000000901fffff
>    1: 0x0000000090200000..0x00000000905fffff
>    2: 0x0000000090600000..0x0000000092ffffff
>    3: 0x0000000093000000..0x00000000973fffff
>    4: 0x0000000097400000..0x00000000b71fffff
>    5: 0x00000000c0000000..0x00000000dfffffff
>    6: 0x00000000e2500000..0x00000000f87fffff
>    7: 0x00000000f8800000..0x00000000fa7fffff
>    8: 0x00000000fa800000..0x00000000fd3effff
>    9: 0x00000000fd3f0000..0x00000000fd3fefff
>   10: 0x00000000fd3ff000..0x00000000fd7fffff
>   11: 0x00000000fd800000..0x00000000fd901fff
>   12: 0x00000000fd902000..0x00000000fd909fff
>   13: 0x00000000fd90a000..0x00000000fd90bfff
>   14: 0x00000000fd90c000..0x00000000ffffffff
>   15: 0x0000000880000000..0x0000000affffffff
> 
> So we can add flags and nid to this debugfs.
> 
> For example,
> cat /sys/kernel/debug/memblock/memory
> cnt     base..end       flags   nid

These markers ^^^ are not aligned properly, and also might not be
required as well.

> 0:      0x0000000080000000..0x00000000901fffff  0x0     0x0
> 1:      0x0000000090200000..0x00000000905fffff  0x4     0x0
> 2:      0x0000000090600000..0x0000000092ffffff  0x0     0x0
> 3:      0x0000000093000000..0x00000000973fffff  0x4     0x0
> 4:      0x0000000097400000..0x00000000b71fffff  0x0     0x0
> 5:      0x00000000c0000000..0x00000000dfffffff  0x0     0x0
> 6:      0x00000000e2500000..0x00000000f87fffff  0x0     0x0
> 7:      0x00000000f8800000..0x00000000fa7fffff  0x4     0x0
> 8:      0x00000000fa800000..0x00000000fd3effff  0x0     0x0
> 9:      0x00000000fd3f0000..0x00000000fd3fefff  0x4     0x0
> 10:     0x00000000fd3ff000..0x00000000fd7fffff  0x0     0x0
> 11:     0x00000000fd800000..0x00000000fd901fff  0x4     0x0
> 12:     0x00000000fd902000..0x00000000fd909fff  0x0     0x0
> 13:     0x00000000fd90a000..0x00000000fd90bfff  0x4     0x0
> 14:     0x00000000fd90c000..0x00000000ffffffff  0x0     0x0
> 15:     0x0000000880000000..0x0000000affffffff  0x0     0x0
> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> Reviewed-by: Tarun Sahu <tsahu@linux.ibm.com>
> ---
>  mm/memblock.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 511d4783dcf1..b36fb6b31e0f 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2144,12 +2144,16 @@ static int memblock_debug_show(struct seq_file *m, void *private)
>  	int i;
>  	phys_addr_t end;
>  
> +	seq_puts(m, "cnt\tbase..end\tflags\tnid\n");

Please drop this.

> +
>  	for (i = 0; i < type->cnt; i++) {
>  		reg = &type->regions[i];
>  		end = reg->base + reg->size - 1;
>  
> -		seq_printf(m, "%4d: ", i);
> -		seq_printf(m, "%pa..%pa\n", &reg->base, &end);
> +		seq_printf(m, "%d:\t", i);

Why drop the existing %4d formatting qualifier ?

> +		seq_printf(m, "%pa..%pa\t", &reg->base, &end);
> +		seq_printf(m, "0x%x\t", reg->flags);

Should there be ORed string values for each memblock flag ?

enum memblock_flags {
        MEMBLOCK_NONE           = 0x0,  /* No special request */
        MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
        MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
        MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct mapping */
        MEMBLOCK_DRIVER_MANAGED = 0x8,  /* always detected via a driver */
};

Something like NN | HT | MR | NM | DM ?

> +		seq_printf(m, "0x%x\n", memblock_get_region_node(reg));
>  	}
>  	return 0;
>  }

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

[Mike Rapoport]

On Wed, May 17, 2023 at 11:37:25AM +0530, Anshuman Khandual wrote:
> 
> On 5/17/23 08:27, Yuwei Guan wrote:
> > Currently, the memblock debugfs can display the count of memblock_type and
> > the base and end of the reg. However, when the following scenario occurs,
> 
> scenarios where the memblock flags or nid varies inside a single PA range ?
> I guess the commit message description here can be improved to accommodate
> such details.
> 
> > the information in the existing debugfs cannot make it clear why the
> > address is not consecutive.
> > 
> > For example,
> > cat /sys/kernel/debug/memblock/memory
> >    0: 0x0000000080000000..0x00000000901fffff
> >    1: 0x0000000090200000..0x00000000905fffff
> >    2: 0x0000000090600000..0x0000000092ffffff
> >    3: 0x0000000093000000..0x00000000973fffff
> >    4: 0x0000000097400000..0x00000000b71fffff
> >    5: 0x00000000c0000000..0x00000000dfffffff
> >    6: 0x00000000e2500000..0x00000000f87fffff
> >    7: 0x00000000f8800000..0x00000000fa7fffff
> >    8: 0x00000000fa800000..0x00000000fd3effff
> >    9: 0x00000000fd3f0000..0x00000000fd3fefff
> >   10: 0x00000000fd3ff000..0x00000000fd7fffff
> >   11: 0x00000000fd800000..0x00000000fd901fff
> >   12: 0x00000000fd902000..0x00000000fd909fff
> >   13: 0x00000000fd90a000..0x00000000fd90bfff
> >   14: 0x00000000fd90c000..0x00000000ffffffff
> >   15: 0x0000000880000000..0x0000000affffffff
> > 
> > So we can add flags and nid to this debugfs.
> > 
> > For example,
> > cat /sys/kernel/debug/memblock/memory
> > cnt     base..end       flags   nid
> 
> These markers ^^^ are not aligned properly, and also might not be
> required as well.
> 
> > 0:      0x0000000080000000..0x00000000901fffff  0x0     0x0
> > 1:      0x0000000090200000..0x00000000905fffff  0x4     0x0
> > 2:      0x0000000090600000..0x0000000092ffffff  0x0     0x0
> > 3:      0x0000000093000000..0x00000000973fffff  0x4     0x0
> > 4:      0x0000000097400000..0x00000000b71fffff  0x0     0x0
> > 5:      0x00000000c0000000..0x00000000dfffffff  0x0     0x0
> > 6:      0x00000000e2500000..0x00000000f87fffff  0x0     0x0
> > 7:      0x00000000f8800000..0x00000000fa7fffff  0x4     0x0
> > 8:      0x00000000fa800000..0x00000000fd3effff  0x0     0x0
> > 9:      0x00000000fd3f0000..0x00000000fd3fefff  0x4     0x0
> > 10:     0x00000000fd3ff000..0x00000000fd7fffff  0x0     0x0
> > 11:     0x00000000fd800000..0x00000000fd901fff  0x4     0x0
> > 12:     0x00000000fd902000..0x00000000fd909fff  0x0     0x0
> > 13:     0x00000000fd90a000..0x00000000fd90bfff  0x4     0x0
> > 14:     0x00000000fd90c000..0x00000000ffffffff  0x0     0x0
> > 15:     0x0000000880000000..0x0000000affffffff  0x0     0x0
> > 
> > Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> > Reviewed-by: Tarun Sahu <tsahu@linux.ibm.com>
> > ---
> >  mm/memblock.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 511d4783dcf1..b36fb6b31e0f 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -2144,12 +2144,16 @@ static int memblock_debug_show(struct seq_file *m, void *private)
> >  	int i;
> >  	phys_addr_t end;
> >  
> > +	seq_puts(m, "cnt\tbase..end\tflags\tnid\n");
> 
> Please drop this.
> 
> > +
> >  	for (i = 0; i < type->cnt; i++) {
> >  		reg = &type->regions[i];
> >  		end = reg->base + reg->size - 1;
> >  
> > -		seq_printf(m, "%4d: ", i);
> > -		seq_printf(m, "%pa..%pa\n", &reg->base, &end);
> > +		seq_printf(m, "%d:\t", i);
> 
> Why drop the existing %4d formatting qualifier ?
> 
> > +		seq_printf(m, "%pa..%pa\t", &reg->base, &end);
> > +		seq_printf(m, "0x%x\t", reg->flags);
> 
> Should there be ORed string values for each memblock flag ?
> 
> enum memblock_flags {
>         MEMBLOCK_NONE           = 0x0,  /* No special request */
>         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
>         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
>         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct mapping */
>         MEMBLOCK_DRIVER_MANAGED = 0x8,  /* always detected via a driver */
> };
> 
> Something like NN | HT | MR | NM | DM ?

These are not less cryptic than numbers :)
Most of them are mutually exclusive, so maybe just spell them out fully,
just shorten DRIVER_MANAGED to DRV_MNG?
And make the flags dump the last to keep columns nicely aligned.
 
> > +		seq_printf(m, "0x%x\n", memblock_get_region_node(reg));
> >  	}
> >  	return 0;
> >  }

-- 
Sincerely yours,
Mike.

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

[Mike Rapoport]

On Wed, May 17, 2023 at 10:57:47AM +0800, Yuwei Guan wrote:
> Currently, the memblock debugfs can display the count of memblock_type and
> the base and end of the reg. However, when the following scenario occurs,
> the information in the existing debugfs cannot make it clear why the
> address is not consecutive.

...
 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 511d4783dcf1..b36fb6b31e0f 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2144,12 +2144,16 @@ static int memblock_debug_show(struct seq_file *m, void *private)
>  	int i;
>  	phys_addr_t end;
>  
> +	seq_puts(m, "cnt\tbase..end\tflags\tnid\n");
> +
>  	for (i = 0; i < type->cnt; i++) {
>  		reg = &type->regions[i];
>  		end = reg->base + reg->size - 1;
>  
> -		seq_printf(m, "%4d: ", i);
> -		seq_printf(m, "%pa..%pa\n", &reg->base, &end);
> +		seq_printf(m, "%d:\t", i);
> +		seq_printf(m, "%pa..%pa\t", &reg->base, &end);
> +		seq_printf(m, "0x%x\t", reg->flags);
> +		seq_printf(m, "0x%x\n", memblock_get_region_node(reg));

Please use "%4d" for nid.

>  	}
>  	return 0;
>  }
> -- 
> 2.34.1
> 

-- 
Sincerely yours,
Mike.

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

[Yuwei Guan]

Mike Rapoport <rppt@kernel.org> 于2023年5月17日周三 21:42写道：
>
> On Wed, May 17, 2023 at 11:37:25AM +0530, Anshuman Khandual wrote:
> >
> > On 5/17/23 08:27, Yuwei Guan wrote:
> > > Currently, the memblock debugfs can display the count of memblock_type and
> > > the base and end of the reg. However, when the following scenario occurs,
> >
> > scenarios where the memblock flags or nid varies inside a single PA range ?
> > I guess the commit message description here can be improved to accommodate
> > such details.
> >
> > > the information in the existing debugfs cannot make it clear why the
> > > address is not consecutive.
> > >
> > > For example,
> > > cat /sys/kernel/debug/memblock/memory
> > >    0: 0x0000000080000000..0x00000000901fffff
> > >    1: 0x0000000090200000..0x00000000905fffff
> > >    2: 0x0000000090600000..0x0000000092ffffff
> > >    3: 0x0000000093000000..0x00000000973fffff
> > >    4: 0x0000000097400000..0x00000000b71fffff
> > >    5: 0x00000000c0000000..0x00000000dfffffff
> > >    6: 0x00000000e2500000..0x00000000f87fffff
> > >    7: 0x00000000f8800000..0x00000000fa7fffff
> > >    8: 0x00000000fa800000..0x00000000fd3effff
> > >    9: 0x00000000fd3f0000..0x00000000fd3fefff
> > >   10: 0x00000000fd3ff000..0x00000000fd7fffff
> > >   11: 0x00000000fd800000..0x00000000fd901fff
> > >   12: 0x00000000fd902000..0x00000000fd909fff
> > >   13: 0x00000000fd90a000..0x00000000fd90bfff
> > >   14: 0x00000000fd90c000..0x00000000ffffffff
> > >   15: 0x0000000880000000..0x0000000affffffff
> > >
> > > So we can add flags and nid to this debugfs.
> > >
> > > For example,
> > > cat /sys/kernel/debug/memblock/memory
> > > cnt     base..end       flags   nid
> >
> > These markers ^^^ are not aligned properly, and also might not be
> > required as well.
> >
> > > 0:      0x0000000080000000..0x00000000901fffff  0x0     0x0
> > > 1:      0x0000000090200000..0x00000000905fffff  0x4     0x0
> > > 2:      0x0000000090600000..0x0000000092ffffff  0x0     0x0
> > > 3:      0x0000000093000000..0x00000000973fffff  0x4     0x0
> > > 4:      0x0000000097400000..0x00000000b71fffff  0x0     0x0
> > > 5:      0x00000000c0000000..0x00000000dfffffff  0x0     0x0
> > > 6:      0x00000000e2500000..0x00000000f87fffff  0x0     0x0
> > > 7:      0x00000000f8800000..0x00000000fa7fffff  0x4     0x0
> > > 8:      0x00000000fa800000..0x00000000fd3effff  0x0     0x0
> > > 9:      0x00000000fd3f0000..0x00000000fd3fefff  0x4     0x0
> > > 10:     0x00000000fd3ff000..0x00000000fd7fffff  0x0     0x0
> > > 11:     0x00000000fd800000..0x00000000fd901fff  0x4     0x0
> > > 12:     0x00000000fd902000..0x00000000fd909fff  0x0     0x0
> > > 13:     0x00000000fd90a000..0x00000000fd90bfff  0x4     0x0
> > > 14:     0x00000000fd90c000..0x00000000ffffffff  0x0     0x0
> > > 15:     0x0000000880000000..0x0000000affffffff  0x0     0x0
> > >
> > > Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> > > Reviewed-by: Tarun Sahu <tsahu@linux.ibm.com>
> > > ---
> > >  mm/memblock.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 511d4783dcf1..b36fb6b31e0f 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -2144,12 +2144,16 @@ static int memblock_debug_show(struct seq_file *m, void *private)
> > >     int i;
> > >     phys_addr_t end;
> > >
> > > +   seq_puts(m, "cnt\tbase..end\tflags\tnid\n");
> >
> > Please drop this.
> >
> > > +
> > >     for (i = 0; i < type->cnt; i++) {
> > >             reg = &type->regions[i];
> > >             end = reg->base + reg->size - 1;
> > >
> > > -           seq_printf(m, "%4d: ", i);
> > > -           seq_printf(m, "%pa..%pa\n", &reg->base, &end);
> > > +           seq_printf(m, "%d:\t", i);
> >
> > Why drop the existing %4d formatting qualifier ?
> >
> > > +           seq_printf(m, "%pa..%pa\t", &reg->base, &end);
> > > +           seq_printf(m, "0x%x\t", reg->flags);
> >
> > Should there be ORed string values for each memblock flag ?
> >
> > enum memblock_flags {
> >         MEMBLOCK_NONE           = 0x0,  /* No special request */
> >         MEMBLOCK_HOTPLUG        = 0x1,  /* hotpluggable region */
> >         MEMBLOCK_MIRROR         = 0x2,  /* mirrored region */
> >         MEMBLOCK_NOMAP          = 0x4,  /* don't add to kernel direct mapping */
> >         MEMBLOCK_DRIVER_MANAGED = 0x8,  /* always detected via a driver */
> > };
> >
> > Something like NN | HT | MR | NM | DM ?
>
> These are not less cryptic than numbers :)
> Most of them are mutually exclusive, so maybe just spell them out fully,
> just shorten DRIVER_MANAGED to DRV_MNG?
> And make the flags dump the last to keep columns nicely aligned.
>

Ok, I will update them in the v3 patch soon.

Thanks
> > > +           seq_printf(m, "0x%x\n", memblock_get_region_node(reg));
> > >     }
> > >     return 0;
> > >  }
>
> --
> Sincerely yours,
> Mike.