Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Concurrent, unprotected access to the global `tierable_nodes` nodemask can result in a torn read. --
--- Patch [29]: [PATCH v5 29/36] mm: add NODE_PRIVATE_CAP_DEMOTION for private-node tiering demotion --- commit e4de60011eb318a533982e5d4b896e77cc0ad141 Author: Gregory Price <[email protected]> mm: add NODE_PRIVATE_CAP_DEMOTION for private-node tiering demotion This commit introduces NODE_PRIVATE_CAP_DEMOTION, allowing private memory nodes to be included in the tiering and demotion hierarchy. By opting into this capability, memory-tiers will include the private node in demotion paths, allowing page reclaim to target it. > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c > index 25e121851b586..c673080d153e4 100644 > --- a/mm/memory-tiers.c > +++ b/mm/memory-tiers.c > @@ -317,6 +318,21 @@ void node_get_allowed_targets(pg_data_t *pgdat, > nodemask_t *targets) > rcu_read_unlock(); > } > > +/* Tiering set: N_MEMORY | (N_MEMORY_PRIVATE w/ CAP_DEMOTION) */ > +static nodemask_t tierable_nodes; > + > +static void update_tierable_nodes(void) > +{ > + int node; > + > + lockdep_assert_held_once(&memory_tier_lock); > + > + tierable_nodes = node_states[N_MEMORY]; > + for_each_node_state(node, N_MEMORY_PRIVATE) > + if (node_allows_demotion(node)) > + node_set(node, tierable_nodes); > +} > + [ ... ] > @@ -330,7 +346,7 @@ void node_get_allowed_targets(pg_data_t *pgdat, > nodemask_t *targets) > int next_demotion_node(int node, const nodemask_t *allowed_mask) > { > struct demotion_nodes *nd; > - nodemask_t mask; > + nodemask_t mask, tierable; > > if (!node_demotion) > return NUMA_NO_NODE; [ ... ] > @@ -370,7 +386,8 @@ int next_demotion_node(int node, const nodemask_t > *allowed_mask) > * closest demotion target. > */ > nodes_complement(mask, *allowed_mask); > - return find_next_best_node_in(node, &mask, &node_states[N_MEMORY]); > + tierable = tierable_nodes; [Severity: Medium] Could this lockless copy result in a torn read of the tierable_nodes nodemask? In update_tierable_nodes(), the global tierable_nodes mask is updated via a non-atomic struct assignment and iterative node_set() modifications. Here in next_demotion_node(), the mask is copied locklessly to the stack during the reclaim hotpath. Since nodemask_t can be up to 128 bytes depending on MAX_NUMNODES, concurrent updates from memory hotplug could cause next_demotion_node() to see a partially updated mask. This could cause page reclaim to transiently skip valid private nodes or behave suboptimally during demotion. > + return find_next_best_node_in(node, &mask, &tierable); > } > > static void disable_all_demotion_targets(void) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=29
