2021-08-30 12:18:22

by Bharata B Rao

Subject: [FIX PATCH 1/2] mm/page_alloc: Print node fallback order

Print information message about the allocation fallback order
for each NUMA node during boot.

No functional changes here. This makes it easier to illustrate
the problem in the node fallback list generation, which the
next patch fixes.

Signed-off-by: Bharata B Rao <[email protected]>
---
mm/page_alloc.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index eeb3a9cb36bb..22f7ad6ec11c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6277,6 +6277,10 @@ static void build_zonelists(pg_data_t *pgdat)

build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
build_thisnode_zonelists(pgdat);
+ pr_info("Fallback order for Node %d: ", local_node);
+ for (node = 0; node < nr_nodes; node++)
+ pr_cont("%d ", node_order[node]);
+ pr_cont("\n");
}

#ifdef CONFIG_HAVE_MEMORYLESS_NODES
--
2.25.1

2021-08-30 12:27:53

by Mel Gorman

Subject: Re: [FIX PATCH 1/2] mm/page_alloc: Print node fallback order

On Mon, Aug 30, 2021 at 05:46:02PM +0530, Bharata B Rao wrote:
> Print information message about the allocation fallback order
> for each NUMA node during boot.
>
> No functional changes here. This makes it easier to illustrate
> the problem in the node fallback list generation, which the
> next patch fixes.
>
> Signed-off-by: Bharata B Rao <[email protected]>

While the message is just informational, I think it's possible that the
bug could have been found earlier if this message was present so...

Acked-by: Mel Gorman <[email protected]>

--
Mel Gorman
SUSE Labs

2021-09-03 04:32:38

by Anshuman Khandual

Subject: Re: [FIX PATCH 1/2] mm/page_alloc: Print node fallback order

On 8/30/21 5:46 PM, Bharata B Rao wrote:
> Print information message about the allocation fallback order
> for each NUMA node during boot.
>
> No functional changes here. This makes it easier to illustrate
> the problem in the node fallback list generation, which the
> next patch fixes.
>
> Signed-off-by: Bharata B Rao <[email protected]>
> ---
> mm/page_alloc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index eeb3a9cb36bb..22f7ad6ec11c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6277,6 +6277,10 @@ static void build_zonelists(pg_data_t *pgdat)
>
> build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
> build_thisnode_zonelists(pgdat);
> + pr_info("Fallback order for Node %d: ", local_node);
> + for (node = 0; node < nr_nodes; node++)
> + pr_cont("%d ", node_order[node]);
> + pr_cont("\n");
> }
>
> #ifdef CONFIG_HAVE_MEMORYLESS_NODES
>

Node allocation preference sequence for a given accessing node is an
important information on large systems. This information during boot
and (hotplug --> online sequence) will help the user.

Reviewed-by: Anshuman Khandual <[email protected]>

2021-09-03 04:33:28

by Bharata B Rao

Subject: Re: [FIX PATCH 1/2] mm/page_alloc: Print node fallback order

On 9/3/2021 9:45 AM, Anshuman Khandual wrote:
>
>
> On 8/30/21 5:46 PM, Bharata B Rao wrote:
>> Print information message about the allocation fallback order
>> for each NUMA node during boot.
>>
>> No functional changes here. This makes it easier to illustrate
>> the problem in the node fallback list generation, which the
>> next patch fixes.
>>
>> Signed-off-by: Bharata B Rao <[email protected]>
>> ---
>> mm/page_alloc.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index eeb3a9cb36bb..22f7ad6ec11c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6277,6 +6277,10 @@ static void build_zonelists(pg_data_t *pgdat)
>>
>> build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
>> build_thisnode_zonelists(pgdat);
>> + pr_info("Fallback order for Node %d: ", local_node);
>> + for (node = 0; node < nr_nodes; node++)
>> + pr_cont("%d ", node_order[node]);
>> + pr_cont("\n");
>> }
>>
>> #ifdef CONFIG_HAVE_MEMORYLESS_NODES
>>
>
> A small nit, checkpatch.pl throws up an warning. Should this use
> pr_info() instead ?
>
> WARNING: Avoid logging continuation uses where feasible
> #29: FILE: mm/page_alloc.c:6282:
> + pr_cont("%d ", node_order[node]);
>
> WARNING: Avoid logging continuation uses where feasible
> #30: FILE: mm/page_alloc.c:6283:
> + pr_cont("\n");
>
> total: 0 errors, 2 warnings, 10 lines checked

Yes, I am aware of this, but then it made sense for the fallback list
to be printed in one line continuously. Hence used pr_cont().

Regards,
Bharata.
>

2021-09-03 04:41:09

by Anshuman Khandual

Subject: Re: [FIX PATCH 1/2] mm/page_alloc: Print node fallback order

On 8/30/21 5:46 PM, Bharata B Rao wrote:
> Print information message about the allocation fallback order
> for each NUMA node during boot.
>
> No functional changes here. This makes it easier to illustrate
> the problem in the node fallback list generation, which the
> next patch fixes.
>
> Signed-off-by: Bharata B Rao <[email protected]>
> ---
> mm/page_alloc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index eeb3a9cb36bb..22f7ad6ec11c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6277,6 +6277,10 @@ static void build_zonelists(pg_data_t *pgdat)
>
> build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
> build_thisnode_zonelists(pgdat);
> + pr_info("Fallback order for Node %d: ", local_node);
> + for (node = 0; node < nr_nodes; node++)
> + pr_cont("%d ", node_order[node]);
> + pr_cont("\n");
> }
>
> #ifdef CONFIG_HAVE_MEMORYLESS_NODES
>

A small nit, checkpatch.pl throws up an warning. Should this use
pr_info() instead ?

WARNING: Avoid logging continuation uses where feasible
#29: FILE: mm/page_alloc.c:6282:
+ pr_cont("%d ", node_order[node]);

WARNING: Avoid logging continuation uses where feasible
#30: FILE: mm/page_alloc.c:6283:
+ pr_cont("\n");

total: 0 errors, 2 warnings, 10 lines checked