Re: [PATCH 6/9] mm, page_alloc: simplify zonelist initialization
flat view
From: Mel Gorman <mgorman@suse.de>
Date: 2017-07-14 14:18:28
Also in:
lkml
On Fri, Jul 14, 2017 at 03:02:42PM +0200, Michal Hocko wrote:
quoted
It *might* be safer given the next patch to zero out the remainder of the _zonerefs to that there is no combination of node add/remove that has an iterator working with a semi-valid _zoneref which is beyond the last correct value. It *should* be safe as the very last entry will always be null but if you don't zero it out, it is possible for iterators to be working beyond the "end" of the zonelist for a short window.yes that is true but there will always be terminating NULL zone and I found that acceptable. It is basically the same thing as accessing an empty zone or a zone twice. Or do you think this is absolutely necessary to handle?
I don't think it's absolutely necessary. While you could construct some odd behaviour for iterators currently past the end of the list, they would eventually encounter a NULL.
quoted hunk ↗ jump to hunk
quoted
Otherwise think it's ok including my stupid comment about node_order stack usage.What do you think about this on top? ---diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 49bade7ff049..3b98524c04ec 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c@@ -4913,20 +4913,21 @@ static int find_next_best_node(int node, nodemask_t *used_node_mask) * This results in maximum locality--normal zone overflows into local * DMA zone, if any--but risks exhausting DMA zone. */ -static void build_zonelists_in_node_order(pg_data_t *pgdat, int *node_order) +static void build_zonelists_in_node_order(pg_data_t *pgdat, int *node_order, + unsigned nr_nodes) { struct zonelist *zonelist; - int i, zoneref_idx = 0; + int i, nr_zones = 0; zonelist = &pgdat->node_zonelists[ZONELIST_FALLBACK]; - for (i = 0; i < MAX_NUMNODES; i++) { + for (i = 0; i < nr_nodes; i++) {
The first iteration is then -- for (i = 0; i < 0; i++) Fairly sure that's not what you meant.
quoted hunk ↗ jump to hunk
pg_data_t *node = NODE_DATA(node_order[i]); - zoneref_idx = build_zonelists_node(node, zonelist, zoneref_idx); + nr_zones = build_zonelists_node(node, zonelist, nr_zones);
I meant converting build_zonelists_node and passing in &nr_zones and returning false when an empty node is encountered. In this context, it's also not about zones, it really is nr_zonerefs. Rename nr_zones in build_zonelists_node as well. -- Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>