linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] memblock: Add flags and nid info in memblock debugfs
@ 2023-05-19 10:53 Yuwei Guan
  2023-05-23  5:53 ` Anshuman Khandual
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Yuwei Guan @ 2023-05-19 10:53 UTC (permalink / raw)
  To: rppt, akpm, tsahu, anshuman.khandual; +Cc: linux-mm, linux-kernel, Yuwei Guan

Currently, the memblock debugfs can display the count of memblock_type and
the base and end of the reg. However, when memblock_mark_*() or
memblock_set_node() is executed on some range, 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
   0: 0x0000000080000000..0x00000000901fffff    0 NONE
   1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
   2: 0x0000000090600000..0x0000000092ffffff    0 NONE
   3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
   4: 0x0000000097400000..0x00000000b71fffff    0 NONE
   5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
   6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
   7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
   8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
   9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
  10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
  11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
  12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
  13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
  14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
  15: 0x0000000880000000..0x0000000affffffff    0 NONE

Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
---
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));
+		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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
@ 2023-05-23  5:53 ` Anshuman Khandual
  2023-05-23 11:36 ` Kefeng Wang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2023-05-23  5:53 UTC (permalink / raw)
  To: Yuwei Guan, rppt, akpm, tsahu; +Cc: linux-mm, linux-kernel



On 5/19/23 16:23, Yuwei Guan wrote:
> Currently, the memblock debugfs can display the count of memblock_type and
> the base and end of the reg. However, when memblock_mark_*() or
> memblock_set_node() is executed on some range, 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
>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>

LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> ---
> 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));
> +		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;
>  }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
  2023-05-23  5:53 ` Anshuman Khandual
@ 2023-05-23 11:36 ` Kefeng Wang
  2023-05-23 16:05 ` Mike Rapoport
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kefeng Wang @ 2023-05-23 11:36 UTC (permalink / raw)
  To: Yuwei Guan, rppt, akpm, tsahu, anshuman.khandual; +Cc: linux-mm, linux-kernel



On 2023/5/19 18:53, Yuwei Guan wrote:
> Currently, the memblock debugfs can display the count of memblock_type and
> the base and end of the reg. However, when memblock_mark_*() or
> memblock_set_node() is executed on some range, 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
>     0: 0x0000000080000000..0x00000000901fffff    0 NONE
>     1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>     2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>     3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>     4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>     5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>     6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>     7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>     8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>     9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>    10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>    11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>    12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>    13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>    14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>    15: 0x0000000880000000..0x0000000affffffff    0 NONE

It's useful for debug memblock, thanks,

Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>

I also found memblock reserved type's node is always wrong,
from memblock=debug, and read /sys/kernel/debug/memblock/reserved
return bad node info, I will send a patch.

> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> ---
> 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));
> +		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;
>   }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
  2023-05-23  5:53 ` Anshuman Khandual
  2023-05-23 11:36 ` Kefeng Wang
@ 2023-05-23 16:05 ` Mike Rapoport
  2023-05-24  4:12   ` Anshuman Khandual
  2023-05-24  8:57 ` Mike Rapoport
  2023-05-27 10:37 ` Mike Rapoport
  4 siblings, 1 reply; 10+ messages in thread
From: Mike Rapoport @ 2023-05-23 16:05 UTC (permalink / raw)
  To: Yuwei Guan; +Cc: akpm, tsahu, anshuman.khandual, linux-mm, linux-kernel

Hi,

On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
> memblock_set_node() is executed on some range, 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
>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> ---
> 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));
> +		if (reg->flags) {
> +			for (j = 0; j < count; j++) {
> +				if (reg->flags & (1U << j)) {
> +					seq_printf(m, "%s\n", flagname[j]);
> +					break;

The flags are mostly mutually exclusive because it's unlikely they are used
together, but not because there are some restrictions on possible flags
combinations. So generally it's possible to have multiple flags set on the
same region.

Sorry, I missed that in v3.

> +				}
> +			}
> +			if (j == count)
> +				seq_printf(m, "%s\n", "UNKNOWN");
> +		} else {
> +			seq_printf(m, "%s\n", "NONE");
> +		}
>  	}
>  	return 0;
>  }
> -- 
> 2.34.1
> 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-23 16:05 ` Mike Rapoport
@ 2023-05-24  4:12   ` Anshuman Khandual
  2023-05-24  8:35     ` Mike Rapoport
  0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2023-05-24  4:12 UTC (permalink / raw)
  To: Mike Rapoport, Yuwei Guan; +Cc: akpm, tsahu, linux-mm, linux-kernel



On 5/23/23 21:35, Mike Rapoport wrote:
> Hi,
> 
> On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
>> memblock_set_node() is executed on some range, 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
>>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
>>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
>>
>> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
>> ---
>> 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));
>> +		if (reg->flags) {
>> +			for (j = 0; j < count; j++) {
>> +				if (reg->flags & (1U << j)) {
>> +					seq_printf(m, "%s\n", flagname[j]);
>> +					break;
> 
> The flags are mostly mutually exclusive because it's unlikely they are used
> together, but not because there are some restrictions on possible flags
> combinations. So generally it's possible to have multiple flags set on the
> same region.

But do we really need to account for that as of now, when no memblock region
carries more than a single flag ? But in that case all flags here need to be
printed with "|" in between ?

> 
> Sorry, I missed that in v3.
> 
>> +				}
>> +			}
>> +			if (j == count)
>> +				seq_printf(m, "%s\n", "UNKNOWN");
>> +		} else {
>> +			seq_printf(m, "%s\n", "NONE");
>> +		}
>>  	}
>>  	return 0;
>>  }
>> -- 
>> 2.34.1
>>
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-24  4:12   ` Anshuman Khandual
@ 2023-05-24  8:35     ` Mike Rapoport
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Rapoport @ 2023-05-24  8:35 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: Yuwei Guan, akpm, tsahu, linux-mm, linux-kernel

On Wed, May 24, 2023 at 09:42:40AM +0530, Anshuman Khandual wrote:
> On 5/23/23 21:35, Mike Rapoport wrote:
> > Hi,
> > 
> > On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
> >> memblock_set_node() is executed on some range, 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
> >>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
> >>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
> >>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
> >>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
> >>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
> >>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
> >>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
> >>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
> >>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
> >>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
> >>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
> >>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
> >>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
> >>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
> >>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
> >>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> >>
> >> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> >> ---
> >> 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));
> >> +		if (reg->flags) {
> >> +			for (j = 0; j < count; j++) {
> >> +				if (reg->flags & (1U << j)) {
> >> +					seq_printf(m, "%s\n", flagname[j]);
> >> +					break;
> > 
> > The flags are mostly mutually exclusive because it's unlikely they are used
> > together, but not because there are some restrictions on possible flags
> > combinations. So generally it's possible to have multiple flags set on the
> > same region.
> 
> But do we really need to account for that as of now, when no memblock region
> carries more than a single flag ?

Right now we don't care indeed, I'm a bit concerned of what will happen if
we'd have regions with multiple flags set.

> But in that case all flags here need to be printed with "|" in between ?
 
We could use space rather than "|" for simplicity, but thinking more about
it, I'm inclined to just take this version and we'll deal with multiple
flags if/when required.

> > Sorry, I missed that in v3.
> > 
> >> +				}
> >> +			}
> >> +			if (j == count)
> >> +				seq_printf(m, "%s\n", "UNKNOWN");
> >> +		} else {
> >> +			seq_printf(m, "%s\n", "NONE");
> >> +		}
> >>  	}
> >>  	return 0;
> >>  }
> >> -- 
> >> 2.34.1
> >>
> > 

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
                   ` (2 preceding siblings ...)
  2023-05-23 16:05 ` Mike Rapoport
@ 2023-05-24  8:57 ` Mike Rapoport
  2023-05-27 10:37 ` Mike Rapoport
  4 siblings, 0 replies; 10+ messages in thread
From: Mike Rapoport @ 2023-05-24  8:57 UTC (permalink / raw)
  To: Yuwei Guan; +Cc: akpm, tsahu, anshuman.khandual, linux-mm, linux-kernel

On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
> memblock_set_node() is executed on some range, 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
>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>

Applied, thanks!

> ---
> 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));
> +		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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
                   ` (3 preceding siblings ...)
  2023-05-24  8:57 ` Mike Rapoport
@ 2023-05-27 10:37 ` Mike Rapoport
  2023-05-28 14:38   ` Yuwei Guan
  4 siblings, 1 reply; 10+ messages in thread
From: Mike Rapoport @ 2023-05-27 10:37 UTC (permalink / raw)
  To: Yuwei Guan; +Cc: akpm, tsahu, anshuman.khandual, linux-mm, linux-kernel

Hi Yuwei,

On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
> memblock_set_node() is executed on some range, 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
>    0: 0x0000000080000000..0x00000000901fffff    0 NONE
>    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
>    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
>    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
>    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
>    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
>    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
>    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
>    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
>    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
>   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
>   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
>   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
>   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
>   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
>   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> 
> Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> ---
> 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'. 

> +		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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-27 10:37 ` Mike Rapoport
@ 2023-05-28 14:38   ` Yuwei Guan
  2023-05-28 17:53     ` Mike Rapoport
  0 siblings, 1 reply; 10+ messages in thread
From: Yuwei Guan @ 2023-05-28 14:38 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: akpm, tsahu, anshuman.khandual, linux-mm, linux-kernel, wangkefeng.wang

Mike Rapoport <rppt@kernel.org> 于2023年5月27日周六 18:37写道:
>
> Hi Yuwei,
>
> On Fri, May 19, 2023 at 06:53:21PM +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 memblock_mark_*() or
> > memblock_set_node() is executed on some range, 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
> >    0: 0x0000000080000000..0x00000000901fffff    0 NONE
> >    1: 0x0000000090200000..0x00000000905fffff    0 NOMAP
> >    2: 0x0000000090600000..0x0000000092ffffff    0 NONE
> >    3: 0x0000000093000000..0x00000000973fffff    0 NOMAP
> >    4: 0x0000000097400000..0x00000000b71fffff    0 NONE
> >    5: 0x00000000c0000000..0x00000000dfffffff    0 NONE
> >    6: 0x00000000e2500000..0x00000000f87fffff    0 NONE
> >    7: 0x00000000f8800000..0x00000000fa7fffff    0 NOMAP
> >    8: 0x00000000fa800000..0x00000000fd3effff    0 NONE
> >    9: 0x00000000fd3f0000..0x00000000fd3fefff    0 NOMAP
> >   10: 0x00000000fd3ff000..0x00000000fd7fffff    0 NONE
> >   11: 0x00000000fd800000..0x00000000fd901fff    0 NOMAP
> >   12: 0x00000000fd902000..0x00000000fd909fff    0 NONE
> >   13: 0x00000000fd90a000..0x00000000fd90bfff    0 NOMAP
> >   14: 0x00000000fd90c000..0x00000000ffffffff    0 NONE
> >   15: 0x0000000880000000..0x0000000affffffff    0 NONE
> >
> > Signed-off-by: Yuwei Guan <ssawgyw@gmail.com>
> > ---
> > 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@huawei.com/T/#m6d214d4ea8912b64f5efc9c3f51ae71aa057d1f1

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.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] memblock: Add flags and nid info in memblock debugfs
  2023-05-28 14:38   ` Yuwei Guan
@ 2023-05-28 17:53     ` Mike Rapoport
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Rapoport @ 2023-05-28 17:53 UTC (permalink / raw)
  To: Yuwei Guan
  Cc: akpm, tsahu, anshuman.khandual, linux-mm, linux-kernel, wangkefeng.wang

On Sun, May 28, 2023 at 10:38:03PM +0800, Yuwei Guan wrote:
> Mike Rapoport <rppt@kernel.org> 于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@gmail.com>
> > > ---
> > > 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@huawei.com/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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-05-28 17:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-19 10:53 [PATCH v4] memblock: Add flags and nid info in memblock debugfs Yuwei Guan
2023-05-23  5:53 ` Anshuman Khandual
2023-05-23 11:36 ` Kefeng Wang
2023-05-23 16:05 ` Mike Rapoport
2023-05-24  4:12   ` Anshuman Khandual
2023-05-24  8:35     ` Mike Rapoport
2023-05-24  8:57 ` Mike Rapoport
2023-05-27 10:37 ` Mike Rapoport
2023-05-28 14:38   ` Yuwei Guan
2023-05-28 17:53     ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).