diff mbox series

[2/2] mm: be more verbose about zonelist initialization

Message ID 20190212095343.23315-3-mhocko@kernel.org (mailing list archive)
State New, archived
Headers show
Series x86, numa: always initialize all possible nodes | expand

Commit Message

Michal Hocko Feb. 12, 2019, 9:53 a.m. UTC 
From: Michal Hocko <mhocko@suse.com>

We have seen several bugs where zonelists have not been initialized
properly and it is not really straightforward to track those bugs down.
One way to help a bit at least is to dump zonelists of each node when
they are (re)initialized.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/page_alloc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

kernel test robot Feb. 13, 2019, 12:12 a.m. UTC | #1 
Hi Michal,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/x86-numa-always-initialize-all-possible-nodes/20190213-071628
config: x86_64-randconfig-x016-201906 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/linux/gfp.h:6,
                    from include/linux/mm.h:10,
                    from mm/page_alloc.c:18:
   mm/page_alloc.c: In function 'build_zonelists':
>> mm/page_alloc.c:5423:31: error: 'z' undeclared (first use in this function)
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                                  ^
   include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
          ^
   mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   mm/page_alloc.c:5423:31: note: each undeclared identifier is reported only once for each function it appears in
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                                  ^
   include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
          ^
   mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:25: error: 'zone' undeclared (first use in this function)
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                            ^~~~
   include/linux/mmzone.h:1036:59: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
                                                              ^~~~
   mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/mmzone.h:1036:57: warning: left-hand operand of comma expression has no effect [-Wunused-value]
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
                                                            ^
   include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
     for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/mmzone.h:1038:50: warning: left-hand operand of comma expression has no effect [-Wunused-value]
      z = next_zones_zonelist(++z, highidx, nodemask), \
                                                     ^
   include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
     for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~

vim +/z +5423 mm/page_alloc.c

  5382	
  5383	/*
  5384	 * Build zonelists ordered by zone and nodes within zones.
  5385	 * This results in conserving DMA zone[s] until all Normal memory is
  5386	 * exhausted, but results in overflowing to remote node while memory
  5387	 * may still exist in local DMA zone.
  5388	 */
  5389	
  5390	static void build_zonelists(pg_data_t *pgdat)
  5391	{
  5392		static int node_order[MAX_NUMNODES];
  5393		int node, load, nr_nodes = 0;
  5394		nodemask_t used_mask;
  5395		int local_node, prev_node;
  5396	
  5397		/* NUMA-aware ordering of nodes */
  5398		local_node = pgdat->node_id;
  5399		load = nr_online_nodes;
  5400		prev_node = local_node;
  5401		nodes_clear(used_mask);
  5402	
  5403		memset(node_order, 0, sizeof(node_order));
  5404		while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {
  5405			/*
  5406			 * We don't want to pressure a particular node.
  5407			 * So adding penalty to the first node in same
  5408			 * distance group to make it round-robin.
  5409			 */
  5410			if (node_distance(local_node, node) !=
  5411			    node_distance(local_node, prev_node))
  5412				node_load[node] = load;
  5413	
  5414			node_order[nr_nodes++] = node;
  5415			prev_node = node;
  5416			load--;
  5417		}
  5418	
  5419		build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
  5420		build_thisnode_zonelists(pgdat);
  5421	
  5422		pr_info("node[%d] zonelist: ", pgdat->node_id);
> 5423		for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
  5424			pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
  5425		pr_cont("\n");
  5426	}
  5427	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Feb. 13, 2019, 2:13 a.m. UTC | #2 
Hi Michal,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/x86-numa-always-initialize-all-possible-nodes/20190213-071628
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/gfp.h:6:0,
                    from include/linux/mm.h:10,
                    from mm/page_alloc.c:18:
   mm/page_alloc.c: In function 'build_zonelists':
   mm/page_alloc.c:5423:31: error: 'z' undeclared (first use in this function)
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                                  ^
   include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
          ^
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   mm/page_alloc.c:5423:31: note: each undeclared identifier is reported only once for each function it appears in
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                                  ^
   include/linux/mmzone.h:1036:7: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
          ^
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   mm/page_alloc.c:5423:25: error: 'zone' undeclared (first use in this function)
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
                            ^
   include/linux/mmzone.h:1036:59: note: in definition of macro 'for_each_zone_zonelist_nodemask'
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
                                                              ^~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/mmzone.h:1036:57: warning: left-hand operand of comma expression has no effect [-Wunused-value]
     for (z = first_zones_zonelist(zlist, highidx, nodemask), zone = zonelist_zone(z); \
                                                            ^
>> include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
     for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/mmzone.h:1038:50: warning: left-hand operand of comma expression has no effect [-Wunused-value]
      z = next_zones_zonelist(++z, highidx, nodemask), \
                                                     ^
>> include/linux/mmzone.h:1058:2: note: in expansion of macro 'for_each_zone_zonelist_nodemask'
     for_each_zone_zonelist_nodemask(zone, z, zlist, highidx, NULL)
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> mm/page_alloc.c:5423:2: note: in expansion of macro 'for_each_zone_zonelist'
     for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
     ^~~~~~~~~~~~~~~~~~~~~~

vim +/for_each_zone_zonelist +5423 mm/page_alloc.c

  5382	
  5383	/*
  5384	 * Build zonelists ordered by zone and nodes within zones.
  5385	 * This results in conserving DMA zone[s] until all Normal memory is
  5386	 * exhausted, but results in overflowing to remote node while memory
  5387	 * may still exist in local DMA zone.
  5388	 */
  5389	
  5390	static void build_zonelists(pg_data_t *pgdat)
  5391	{
  5392		static int node_order[MAX_NUMNODES];
  5393		int node, load, nr_nodes = 0;
  5394		nodemask_t used_mask;
  5395		int local_node, prev_node;
  5396	
  5397		/* NUMA-aware ordering of nodes */
  5398		local_node = pgdat->node_id;
  5399		load = nr_online_nodes;
  5400		prev_node = local_node;
  5401		nodes_clear(used_mask);
  5402	
  5403		memset(node_order, 0, sizeof(node_order));
  5404		while ((node = find_next_best_node(local_node, &used_mask)) >= 0) {
  5405			/*
  5406			 * We don't want to pressure a particular node.
  5407			 * So adding penalty to the first node in same
  5408			 * distance group to make it round-robin.
  5409			 */
  5410			if (node_distance(local_node, node) !=
  5411			    node_distance(local_node, prev_node))
  5412				node_load[node] = load;
  5413	
  5414			node_order[nr_nodes++] = node;
  5415			prev_node = node;
  5416			load--;
  5417		}
  5418	
  5419		build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
  5420		build_thisnode_zonelists(pgdat);
  5421	
  5422		pr_info("node[%d] zonelist: ", pgdat->node_id);
> 5423		for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
  5424			pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
  5425		pr_cont("\n");
  5426	}
  5427	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2e097f336126..c30d59f803fb 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5259,6 +5259,11 @@  static void build_zonelists(pg_data_t *pgdat)
 
 	build_zonelists_in_node_order(pgdat, node_order, nr_nodes);
 	build_thisnode_zonelists(pgdat);
+
+	pr_info("node[%d] zonelist: ", pgdat->node_id);
+	for_each_zone_zonelist(zone, z, &pgdat->node_zonelists[ZONELIST_FALLBACK], MAX_NR_ZONES-1)
+		pr_cont("%d:%s ", zone_to_nid(zone), zone->name);
+	pr_cont("\n");
 }
 
 #ifdef CONFIG_HAVE_MEMORYLESS_NODES